All of lore.kernel.org
 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;


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

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03 18:06 [PATCH v2 0/4] iio: Drop use of %hhx and %hx format strings Jonathan Cameron
2021-06-03 18:06 ` [PATCH v2 1/4] iio: si1133: fix format string warnings Jonathan Cameron
2021-06-09 15:27   ` Nathan Chancellor
2021-06-03 18:06 ` [PATCH v2 2/4] iio: light: si1133: Drop remaining uses of %hhx format string Jonathan Cameron
2021-06-09 15:27   ` Nathan Chancellor
2021-06-03 18:06 ` [PATCH v2 3/4] iio: imu: inv_mpu6050: Drop use " Jonathan Cameron
2021-06-09 15:28   ` Nathan Chancellor
2021-06-03 18:06 ` [PATCH v2 4/4] iio: light: si1145: Drop use of %hhx format specifier Jonathan Cameron
2021-06-09 15:29   ` Nathan Chancellor
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
2021-06-09 13:09 ` [PATCH v2 0/4] iio: Drop use of %hhx and %hx format strings Jonathan Cameron
2021-06-09 17:32   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.