From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7A1B88BFD for ; Thu, 26 Oct 2023 22:45:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=none X-Greylist: delayed 354 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Thu, 26 Oct 2023 15:45:05 PDT Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF1B4CC for ; Thu, 26 Oct 2023 15:45:05 -0700 (PDT) Received: from omf03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id EC899120CA5; Thu, 26 Oct 2023 22:39:08 +0000 (UTC) Received: from [HIDDEN] (Authenticated sender: joe@perches.com) by omf03.hostedemail.com (Postfix) with ESMTPA id 229B46000C; Thu, 26 Oct 2023 22:38:40 +0000 (UTC) Message-ID: <8521c712250bcffce5c71e8d2b2574de786d4572.camel@perches.com> Subject: Re: [PATCH next v2 2/3] checkpatch: add ethtool_sprintf rules From: Joe Perches To: Justin Stitt , "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@marvell.com, Dimitris Michailidis , Yisen Zhuang , Salil Mehta , Jesse Brandeburg , Tony Nguyen , Louis Peens , Shannon Nelson , Brett Creeley , drivers@pensando.io, "K. Y. Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Ronak Doshi , VMware PV-Drivers Reviewers , Andy Whitcroft , Dwaipayan Ray , Lukas Bulwahn , Hauke Mehrtens , Andrew Lunn , Florian Fainelli , Vladimir Oltean , =?UTF-8?Q?Ar=C4=B1n=C3=A7_=C3=9CNAL?= , Daniel Golle , Landen Chao , DENG Qingfang , Sean Wang , Matthias Brugger , AngeloGioacchino Del Regno , Linus Walleij , Alvin =?UTF-8?Q?=C5=A0ipraga?= , Wei Fang , Shenwei Wang , Clark Wang , NXP Linux Team , Lars Povlsen , Steen Hegelund , Daniel Machon , UNGLinuxDriver@microchip.com, Jiawen Wu , Mengyuan Lou , Heiner Kallweit , Russell King , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Nick Desaulniers , Nathan Chancellor , Kees Cook , intel-wired-lan@lists.osuosl.org, oss-drivers@corigine.com, linux-hyperv@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, bpf@vger.kernel.org Date: Thu, 26 Oct 2023 15:38:39 -0700 In-Reply-To: <20231026-ethtool_puts_impl-v2-2-0d67cbdd0538@google.com> References: <20231026-ethtool_puts_impl-v2-0-0d67cbdd0538@google.com> <20231026-ethtool_puts_impl-v2-2-0d67cbdd0538@google.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Rspamd-Queue-Id: 229B46000C X-Stat-Signature: mtojnbfkhmj945izba58at9t64m9az5b X-Rspamd-Server: rspamout08 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Session-ID: U2FsdGVkX1/UB5xXHbX1I6Iwr1/WX9S8fJXYcv2DOVQ= X-HE-Tag: 1698359919-976100 X-HE-Meta: U2FsdGVkX1/Amx6RsPXMmp/lJzH0XSfOupRJb1kmsjddDIYEwUvtXmlXvcoG0HQxgfJeu/bgjCSmBW9XNko/mFTeWhpzlQGj+sHcroC4+SQucDoul9/mW561AqHS3bN32ZhIbH4y4PO56Z7gmbgk+jEv1lNmijdnNQn2GDoTkX/tD06ZkKPHkXJTWWOiblBjoSzVelzS2tuc0jq7rbU2KN7wKu2c9SCdLcBX7eDcPHw0DYjtwdWRO7eI8BixY9+TH41p+q5lsfSnzBidMm8GWcRCTJkLQdvf6jjmqEDt8BcNB67WbPmIoEo21hk8mGqMRn8qjdO5KQuIvo+QRzi9TNWL/Qwj5lvUYAvGAivJpjz++odpOBd6Ml2HgJTXQ35CUCCg8zvynD3R6R7WDpQwYoliPG09jddatznky2vFjJO9MiTzTC/t5iwPcMcldvPAUeGRgklQ6A1GlziHr19N0w== On Thu, 2023-10-26 at 21:56 +0000, Justin Stitt wrote: > Add some warnings for using ethtool_sprintf() where a simple > ethtool_puts() would suffice. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -7011,6 +7011,25 @@ sub process { > "Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see:= https://github.com/KSPP/linux/issues/90\n" . $herecurr); > } > =20 > +# ethtool_sprintf uses that should likely be ethtool_puts > + if ($line =3D~ /\bethtool_sprintf\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/= ) { > + if(WARN("ETHTOOL_SPRINTF", > + "Prefer ethtool_puts over ethtool_sprintf with only two arguments\= n" . $herecurr) && > + $fix) { > + $fixed[$fixlinenr] =3D~ s/ethtool_sprintf\s*\(/ethtool_puts\(/; > + } > + } > + > + # use $rawline because $line loses %s via sanitization and thus we can= 't match against it. > + if ($rawline =3D~ /\bethtool_sprintf\s*\(\s*$FuncArg\s*,\s*\"\%s\"\s*,= \s*$FuncArg\s*\)/) { > + if(WARN("ETHTOOL_SPRINTF", > + "Prefer ethtool_puts over ethtool_sprintf with standalone \"%s\" s= pecifier\n" . $herecurr) && > + $fix) { > + $fixed[$fixlinenr] =3D~ s/ethtool_sprintf\s*\(\s*(.*?),.*?,(.*?= )\)/ethtool_puts\($1,$2)/; Thanks, but: This fix wouldn't work if the first argument was itself a function with multiple arguments. And this is whitespace formatted incorrectly. It: o needs a space after if o needs a space after the comma in the fix o needs to use the appropriate amount and tabs for indentation o needs alignment to open parentheses o the backslashes are not required in the fix block Do you want me to push what I wrote in the link below? https://lore.kernel.org/lkml/7eec92d9e72d28e7b5202f41b02a383eb28ddd26.camel= @perches.com/ > + } > + } > + > + > # typecasts on min/max could be min_t/max_t > if ($perl_version_ok && > defined $stat && >=20