Re: [PATCH] checkpatch: add new warning when lookup_symbol_name() is used

[Joe Perches <joe@perches.com>]

On Thu, 2020-12-17 at 18:42 +0100, Helge Deller wrote:
> On 12/17/20 6:27 PM, Joe Perches wrote:
> > On Thu, 2020-12-17 at 18:11 +0100, Helge Deller wrote:
> > > In most cases people use lookup_symbol_name() to resolve a kernel symbol
> > > and then print it via printk().
> > > 
> > > In such cases using the %ps, %pS, %pSR or %pB printk formats are easier
> > > to use and thus should be preferred.
> > []
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -4317,6 +4317,12 @@ sub process {
> > >  			     "LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged\n" . $herecurr);
> > >  		}
> > > 
> > > +# avoid lookup_symbol_name()
> > > +		if ($line =~ /\blookup_symbol_name\b/) {
> > > +			WARN("PREFER_PRINTK_FORMAT",
> > > +			     "If possible prefer %ps or %pS printk format string to print symbol name instead of using lookup_symbol_name()\n" . $herecurr);
> > > +		}
> > > +
> > >  # check for uses of printk_ratelimit
> > >  		if ($line =~ /\bprintk_ratelimit\s*\(/) {
> > >  			WARN("PRINTK_RATELIMITED",
> > 
> > Huh?  nak.
> > 
> > lookup_symbol_name is used in the kernel a grand total of 3 times.
> 
> Yes, there were much more in the past which got fixed by patches I submitted.

Hi Helge.

Much more may be a bit of an overstatement.

I found 3 instances of lookup_symbol_name removals in 2 patches.

commit 36dbca148bf8e3b8658982aa2256bdc7ef040256
-		lookup_symbol_name((ulong)pm_power_off, symname);
-		lookup_symbol_name((ulong)pm_power_off, symname);
commit da88f9b3113620dcd30fc203236aa53d5430ee98
-	if (lookup_symbol_name((unsigned long)sym, symname) < 0)

There's a tension between adding tests and newbies that consider
checkpatch warnings as dicta that must be followed so there would
be patches submitted eventually against the existing correct uses.

So thanks, but given the very few existing all correct uses of
this function and the low probability of new uses I'd prefer not
to apply this.