All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Niklas Söderlund" <niklas.soderlund@corigine.com>
To: Joe Perches <joe@perches.com>
Cc: Andy Whitcroft <apw@canonical.com>, linux-kernel@vger.kernel.org
Subject: False positive for 'Possible unnecessary KERN_ERR' warning in checkpatch
Date: Tue, 14 Sep 2021 18:05:12 +0200	[thread overview]
Message-ID: <YUDIOLTCe0/kzVSW@bismarck.dyn.berto.se> (raw)

Hi Joe,

Maybe you are already aware of this, but in case you are not.

The issue is the checkpatch check for unnecessary KERN_<LEVEL> for log 
functions. If a single line contains a statement that match a log 
function name from $logFunctions, such as foo_err() and the same line 
contains a KERN_<LEVEL> statement the 'WARNING: Possible unnecessary 
KERN_ERR' is triggered. This is true even if the KERN_<LEVEL> statement 
is not part of the arguments to the foo_err() definition that triggers 
the first part of the check.

This can be demonstrated by,

    ./scripts/checkpatch.pl --mailback --git c821e617896e99b8

Where we get (among others) the warning,

    WARNING: Possible unnecessary KERN_ERR
    #38: FILE: drivers/net/ethernet/netronome/nfp/nfp_net.h:63:
    +#define nn_err(nn, fmt, args...)	nn_pr(nn, KERN_ERR, fmt, ## args)

Looking at the code in checkpatch.pl we have,

our $logFunctions = qr{(?x:
        printk(?:_ratelimited|_once|_deferred_once|_deferred|)|
        (?:[a-z0-9]+_){1,2}(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)|
        TP_printk|
        WARN(?:_RATELIMIT|_ONCE|)|
        panic|
        MODULE_[A-Z_]+|
        seq_vprintf|seq_printf|seq_puts
)};

...

# check for logging functions with KERN_<LEVEL>
                if ($line !~ /printk(?:_ratelimited|_once)?\s*\(/ &&
                    $line =~ /\b$logFunctions\s*\(.*\b(KERN_[A-Z]+)\b/) {
                        my $level = $1;
                        if (WARN("UNNECESSARY_KERN_LEVEL",
                                 "Possible unnecessary $level\n" . $herecurr) &&
                            $fix) {
                                $fixed[$fixlinenr] =~ s/\s*$level\s*//;
                        }
                }

Looking at the line from above that triggers the warning,

	#define nn_err(nn, fmt, args...)   nn_pr(nn, KERN_ERR, fmt, ## args)

We see that the warning is triggers by the regexp but that it matches 
the first part on nn_err( and then the second part of on the second 
argument to nn_pr, KERN_ERR. I believe this to be a false positive.

Unfortunately my Perl skills are not good enough to fix the check to only 
look for KERN_[A-Z]+ inside the argument list to the log function name 
that matches the first part of the regexp.

-- 
Regards,
Niklas Söderlund

             reply	other threads:[~2021-09-14 16:05 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-14 16:05 Niklas Söderlund [this message]
2021-09-16  3:46 ` False positive for 'Possible unnecessary KERN_ERR' warning in checkpatch Joe Perches
2021-09-20  9:57   ` Niklas Söderlund

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=YUDIOLTCe0/kzVSW@bismarck.dyn.berto.se \
    --to=niklas.soderlund@corigine.com \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --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.