Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <3c060010a6100aec264f7ad984d2c6920e628a42.camel@perches.com>
Subject: Re: Use of '%#02x' is almost always wrong
From:   Joe Perches <joe@perches.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        kernel-janitors <kernel-janitors@vger.kernel.org>
Date:   Wed, 13 Oct 2021 01:17:30 -0700
In-Reply-To: <20211013061547.GG8429@kadam>
References: <b04a10ea9682c51764053c1be73d4b3be59d1e7f.camel@perches.com>
         <20211013061547.GG8429@kadam>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.40.0-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2021-10-13 at 09:15 +0300, Dan Carpenter wrote:
> On Sun, Oct 10, 2021 at 09:45:30AM -0700, Joe Perches wrote:
> > Perhaps too many developers do not know that using '%#<width>x'
> > in printf output _includes_ the 0x prefix in the output width.
> > 
> 
> This is a good point.  Presumably you're going to add it to checkpatch.pl?

Yeah.  Something like the below:

> I looked at '%#04x' and you would think those would all be printing char
> types but some are printing shorts.  :/  It's harder to deal with that
> because 50% are correct from the tiny sample I looked at.

That'd be difficult as checkpatch doesn't know format argument types.
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c27d2312cfc30..97deae9dafcdc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6703,7 +6703,7 @@ sub process {
 			}
 		}
 
-# check for vsprintf extension %p<foo> misuses
+# check for vsprintf format and extension %p<foo> misuses
 		if ($perl_version_ok &&
 		    defined $stat &&
 		    $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s &&
@@ -6720,6 +6720,18 @@ sub process {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
 
+				# check for %# (0x prefixed) width too small
+				while ($fmt =~ /(%#0?([123])([xX]))/g) {
+					my $hex = $1;
+					my $width = $2;
+					my $case = $3;
+					if (!defined($stat_real)) {
+						$stat_real = get_stat_real($linenr, $lc);
+					}
+					WARN("VSPRINTF_HEX_WIDTH",
+					     "hex specifier '$hex' - output width '$width' does not include the '0$case' prefix, width should be probably be increased\n" . "$here\n$stat_real");
+				}
+				pos($fmt) = 0;
 				while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
 					$specifier = $1;
 					$extension = $2;