Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings

[Philippe Mathieu-Daudé]

+qemu-perl team

On 9/14/20 8:01 AM, Dov Murik wrote:
> According to the coding style document, we should use literal '0x' prefix
> instead of printf's '#' flag (which appears as '%#' or '%0#' in the format
> string).  Add a checkpatch rule to enforce that.
> 
> Note that checkpatch already had a similar rule for trace-events files.
> 
> Example usage:
> 
>   $ scripts/checkpatch.pl --file chardev/baum.c
>   ...
>   ERROR: Don't use '#' flag of printf format ('%#') in format strings, use 
> '0x' prefix instead
>   #366: FILE: chardev/baum.c:366:
>   +            DPRINTF("Broken packet %#2x, tossing\n", req); \
>   ...
>   ERROR: Don't use '#' flag of printf format ('%#') in format strings, use 
> '0x' prefix instead
>   #472: FILE: chardev/baum.c:472:
>   +        DPRINTF("unrecognized request %0#2x\n", req);
>   ...
> 
> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> ---
>  scripts/checkpatch.pl | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index bd3faa154c..6ec2a9f6a1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2891,6 +2891,18 @@ sub process {
>                       }
>               }
>  
> +# check for %# or %0# in printf-style format strings
> +             while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
> +                     my $string = substr($rawline, $-[1], $+[1] - $-[1]);
> +                     $string =~ s/%%/__/g;
> +                     if ($string =~ /(?<!%)%0?#/) {
> +                             ERROR("Don't use '#' flag of printf format " .
> +                                   "('%#') in format strings, use '0x' " .
> +                                   "prefix instead\n" . $herecurr);
> +                             last;
> +                     }
> +             }
> +
>  # QEMU specific tests
>               if ($rawline =~ /\b(?:Qemu|QEmu)\b/) {
>                       ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr);
> 

Thank you for this patch!

What about folding it in the same block?

-- >8 --
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2880,15 +2880,22 @@ sub process {
                                $herecurr);
                }

-# check for %L{u,d,i} in strings
+# format strings checks
                my $string;
                while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
                        $string = substr($rawline, $-[1], $+[1] - $-[1]);
                        $string =~ s/%%/__/g;
+                       # check for %L{u,d,i} in strings
                        if ($string =~ /(?<!%)%L[udi]/) {
                                ERROR("\%Ld/%Lu are not-standard C, use
%lld/%llu\n" . $herecurr);
                                last;
                        }
+                       if ($string =~ /(?<!%)%0?#/) {
+                               ERROR("Don't use '#' flag of printf
format " .
+                                     "('%#') in format strings, use
'0x' " .
+                                     "prefix instead\n" . $herecurr);
+                               last;
+                       }
                }

 # QEMU specific tests
---