linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Jonathan Cameron <jic23@kernel.org>,
	linux-iio@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@kernel.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: General kernel misuse of vsnprintf SPECIAL %#<foo> (was: Re: [PATCH v2 0/4] iio: Drop use of %hhx and %hx format strings)
Date: Thu, 03 Jun 2021 11:58:15 -0700	[thread overview]
Message-ID: <9499203f1e993872b384aabdec59ac223a8ab931.camel@perches.com> (raw)
In-Reply-To: <20210603180612.3635250-1-jic23@kernel.org>

On Thu, 2021-06-03 at 19:06 +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> A wrong use of one of these in
> https://lore.kernel.org/linux-iio/20210514135927.2926482-1-arnd@kernel.org/
> included a reference from Nathan to a patch discouraging the use of
> these strings in general:
> https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/
> 
> I did a quick grep and established we only have a few instances of these in
> IIO anyway, so in the interests of avoiding those existing cases getting
> cut and paste into new drivers, let's just clear them out now.
> 
> Note that patch from Arnd is now also part of this series, due to the
> length specifier related issue Joe raised.
> 
> I have chosen to go with 0x%02x rather than %#04x as I find it more readable.
> 
> V2:
> Use 0x%02x (Joe Perches)
> Include Arnd's original patch, modified for the above.

Hello again.

It looks to me as though %#<foo> is relatively commonly misused in the kernel.

Pehaps for the decimal portion of the format, checkpatch could have some
test for use of non-standard lengths.

Given the use is generally meant for a u8, u16, u32, or u64, perhaps
checkpatch should emit a warning whenever the length is not 4, 6, 10, or 18.

(possible checkpatch patch below)

$ git grep -P -h -o '%#\d+\w+' | sort | uniq -c | sort -rn
    392 %#08x
    238 %#04x
    144 %#02x
    114 %#06x
     92 %#010x
     58 %#010Lx
     55 %#018llx
     47 %#010llx
     45 %#010lx
     38 %#016llx
     27 %#0x
     23 %#2x
     18 %#016lx
     17 %#3lx
     17 %#08lx
     17 %#018Lx
     15 %#3x
     14 %#03x
     10 %#06hx
      9 %#08zx
      8 %#10x
      7 %#16llx
      6 %#8x
      6 %#04X
      6 %#04llx
      6 %#012llx
      5 %#16
      4 %#08llx
      4 %#06llx
      4 %#05x
      4 %#02X
      4 %#016Lx
      3 %#04hx
      3 %#01x
      2 %#6x
      2 %#4x
      2 %#10
      2 %#09x
      2 %#05lx
      1 %#8lx
      1 %#5x
      1 %#5lx
      1 %#2Lx
      1 %#2llx
      1 %#16x
      1 %#16lx
      1 %#12x
      1 %#0x10000
      1 %#0lx
      1 %#08
      1 %#05llx
      1 %#04o
      1 %#04lx
      1 %#03X
      1 %#018lx
      1 %#010zx

---
 scripts/checkpatch.pl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d65334588eb4c..5840f3f2aee6f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6695,6 +6695,31 @@ sub process {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
 
+				while ($fmt =~ /\%#([\d]+)/g) {
+					my $length = $1;
+					my $pref_len;
+					if ($length < 4) {
+						$pref_len = '04';
+					} elsif ($length == 5) {
+						$pref_len = '06';
+					} elsif ($length > 6 && $length < 10) {
+						$pref_len = '010';
+					} elsif ($length > 10 && $length < 18) {
+						$pref_len = '018';
+					} elsif ($length > 18) {
+						$pref_len = '<something else>';
+					}
+					if (defined($pref_len)) {
+						if (!defined($stat_real)) {
+							$stat_real = get_stat_real($linenr, $lc);
+						}
+						WARN("VSPRINTF_SPECIAL_LENGTH",
+						     "Unusual special length '%#$length' in 0x prefixed output, length is usually 2 more than the desired width - perhaps use '%#${pref_len}'\n" . "$here\n$stat_real");
+					}
+				}
+
+				pos($fmt) = 0;
+
 				while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
 					$specifier = $1;
 					$extension = $2;


       reply	other threads:[~2021-06-03 18:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210603180612.3635250-1-jic23@kernel.org>
2021-06-03 18:58 ` Joe Perches [this message]
2021-06-03 19:25   ` General kernel misuse of vsnprintf SPECIAL %#<foo> (was: Re: [PATCH v2 0/4] iio: Drop use of %hhx and %hx format strings) Jonathan Cameron
2021-06-03 19:34     ` Joe Perches
2021-06-04  8:27       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9499203f1e993872b384aabdec59ac223a8ab931.camel@perches.com \
    --to=joe@perches.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=arnd@kernel.org \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).