netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v5 0/3] ethtool: Add ethtool_puts()
@ 2023-12-06 23:16 justinstitt
  2023-12-06 23:16 ` [PATCH net-next v5 1/3] ethtool: Implement ethtool_puts() justinstitt
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: justinstitt @ 2023-12-06 23:16 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shay Agroskin, Arthur Kiyanovski, David Arinzon, Noam Dagan,
	Saeed Bishara, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Dimitris Michailidis, Yisen Zhuang, Salil Mehta, Jesse Brandeburg,
	Tony Nguyen, Louis Peens, Shannon Nelson, Brett Creeley, drivers,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Ronak Doshi,
	VMware PV-Drivers Reviewers, Andy Whitcroft, Joe Perches,
	Dwaipayan Ray, Lukas Bulwahn, Hauke Mehrtens, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, Arınç ÜNAL,
	Daniel Golle, Landen Chao, DENG Qingfang, Sean Wang,
	Matthias Brugger, AngeloGioacchino Del Regno, Linus Walleij,
	Alvin Šipraga, Wei Fang, Shenwei Wang, Clark Wang,
	NXP Linux Team, Lars Povlsen, Steen Hegelund, Daniel Machon,
	UNGLinuxDriver, Jiawen Wu, Mengyuan Lou, Heiner Kallweit,
	Russell King, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend
  Cc: linux-kernel, netdev, Nick Desaulniers, Nathan Chancellor,
	Kees Cook, intel-wired-lan, oss-drivers, linux-hyperv,
	linux-arm-kernel, linux-mediatek, bpf, Justin Stitt

Hi,

This series aims to implement ethtool_puts() and send out a wave 1 of
conversions from ethtool_sprintf(). There's also a checkpatch patch
included to check for the cases listed below.

This was sparked from recent discussion here [1]

The conversions are used in cases where ethtool_sprintf() was being used
with just two arguments:
|       ethtool_sprintf(&data, buffer[i].name);
or when it's used with format string: "%s"
|       ethtool_sprintf(&data, "%s", buffer[i].name);
which both now become:
|       ethtool_puts(&data, buffer[i].name);

The first case commonly triggers a -Wformat-security warning with Clang
due to potential problems with format flags present in the strings [3].

The second is just a bit weird with a plain-ol' "%s".

Changes found with Cocci [4] and grep [5].

[1]: https://lore.kernel.org/all/202310141935.B326C9E@keescook/
[2]: https://lore.kernel.org/all/?q=dfb%3Aethtool_sprintf+AND+f%3Ajustinstitt
[3]: https://lore.kernel.org/all/202310101528.9496539BE@keescook/
[4]: (script authored by Kees w/ modifications from Joe)
@replace_2_args@
expression BUF;
expression VAR;
@@

-       ethtool_sprintf(BUF, VAR)
+       ethtool_puts(BUF, VAR)

@replace_3_args@
expression BUF;
expression VAR;
@@

-       ethtool_sprintf(BUF, "%s", VAR)
+       ethtool_puts(BUF, VAR)

-       ethtool_sprintf(&BUF, "%s", VAR)
+       ethtool_puts(&BUF, VAR)

[5]: $ rg "ethtool_sprintf\(\s*[^,)]+\s*,\s*[^,)]+\s*\)"

Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Changes in v5:
- updated documentation to include info about the lack of a trailing newline
  (Thanks Russell)
- rebased onto mainline
- Link to v4: https://lore.kernel.org/r/20231102-ethtool_puts_impl-v4-0-14e1e9278496@google.com

Changes in v4:
- update documentation to match:
  https://lore.kernel.org/all/20231028192511.100001-1-andrew@lunn.ch/

- Link to v3: https://lore.kernel.org/r/20231027-ethtool_puts_impl-v3-0-3466ac679304@google.com

Changes in v3:
- fix force_speed_maps merge conflict + formatting (thanks Vladimir)
- rebase onto net-next (thanks Andrew, Vladimir)
- change subject (thanks Vladimir)
- fix checkpatch formatting + implementation (thanks Joe)
- Link to v2: https://lore.kernel.org/r/20231026-ethtool_puts_impl-v2-0-0d67cbdd0538@google.com

Changes in v2:
- wrap lines better in replacement (thanks Joe, Kees)
- add --fix to checkpatch (thanks Joe)
- clean up checkpatch formatting (thanks Joe, et al.)
- rebase against next
- Link to v1: https://lore.kernel.org/r/20231025-ethtool_puts_impl-v1-0-6a53a93d3b72@google.com

---
Justin Stitt (3):
      ethtool: Implement ethtool_puts()
      checkpatch: add ethtool_sprintf rules
      net: Convert some ethtool_sprintf() to ethtool_puts()

 drivers/net/dsa/lantiq_gswip.c                     |  2 +-
 drivers/net/dsa/mt7530.c                           |  2 +-
 drivers/net/dsa/qca/qca8k-common.c                 |  2 +-
 drivers/net/dsa/realtek/rtl8365mb.c                |  2 +-
 drivers/net/dsa/realtek/rtl8366-core.c             |  2 +-
 drivers/net/dsa/vitesse-vsc73xx-core.c             |  8 +--
 drivers/net/ethernet/amazon/ena/ena_ethtool.c      |  4 +-
 drivers/net/ethernet/brocade/bna/bnad_ethtool.c    |  2 +-
 drivers/net/ethernet/freescale/fec_main.c          |  4 +-
 .../net/ethernet/fungible/funeth/funeth_ethtool.c  |  8 +--
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c |  2 +-
 .../net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c    |  2 +-
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c   | 65 +++++++++++-----------
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |  6 +-
 drivers/net/ethernet/intel/iavf/iavf_ethtool.c     |  3 +-
 drivers/net/ethernet/intel/ice/ice_ethtool.c       |  9 +--
 drivers/net/ethernet/intel/idpf/idpf_ethtool.c     |  2 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c       |  6 +-
 drivers/net/ethernet/intel/igc/igc_ethtool.c       |  6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c   |  5 +-
 .../net/ethernet/microchip/sparx5/sparx5_ethtool.c |  2 +-
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   | 44 +++++++--------
 drivers/net/ethernet/pensando/ionic/ionic_stats.c  |  4 +-
 drivers/net/ethernet/wangxun/libwx/wx_ethtool.c    |  2 +-
 drivers/net/hyperv/netvsc_drv.c                    |  4 +-
 drivers/net/phy/nxp-tja11xx.c                      |  2 +-
 drivers/net/phy/smsc.c                             |  2 +-
 drivers/net/vmxnet3/vmxnet3_ethtool.c              | 10 ++--
 include/linux/ethtool.h                            | 13 +++++
 net/ethtool/ioctl.c                                |  7 +++
 scripts/checkpatch.pl                              | 19 +++++++
 31 files changed, 139 insertions(+), 112 deletions(-)
---
base-commit: bee0e7762ad2c6025b9f5245c040fcc36ef2bde8
change-id: 20231025-ethtool_puts_impl-a1479ffbc7e0

Best regards,
-- 
Justin Stitt <justinstitt@google.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-12-08 11:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-06 23:16 [PATCH net-next v5 0/3] ethtool: Add ethtool_puts() justinstitt
2023-12-06 23:16 ` [PATCH net-next v5 1/3] ethtool: Implement ethtool_puts() justinstitt
2023-12-07  7:12   ` [Intel-wired-lan] " Przemek Kitszel
2023-12-07 15:27   ` Andrew Lunn
2023-12-08 10:45   ` Madhuri.Sripada
2023-12-06 23:16 ` [PATCH net-next v5 2/3] checkpatch: add ethtool_sprintf rules justinstitt
2023-12-07  7:39   ` [Intel-wired-lan] " Przemek Kitszel
2023-12-06 23:16 ` [PATCH net-next v5 3/3] net: Convert some ethtool_sprintf() to ethtool_puts() justinstitt
2023-12-07  7:19   ` Wei Fang
2023-12-07 15:28   ` Andrew Lunn
2023-12-08  6:33   ` Louis Peens
2023-12-08 11:03   ` Divya.Koppera
2023-12-08 11:00 ` [PATCH net-next v5 0/3] ethtool: Add ethtool_puts() patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).