[Linux-kernel-mentees] [PATCH] checkpatch: add fix for DEPRECATED_VARIABLE

Lukas Bulwahn lukas.bulwahn at gmail.com
Mon Nov 2 07:42:59 UTC 2020



On Mon, 2 Nov 2020, Aditya wrote:

> On 1/11/20 11:51 pm, Aditya Srivastava wrote:
> > The coding style of Makefile has been improved to use certain different
> > variable names as compared to earlier.
> > 
> > E.g., variable name asflags-y should be used instead of EXTRA_AFLAGS, etc
> > 
> > Currently checkpatch.pl warns the user to change variable name with
> > its corresponding upgrade, but does not have a fix option
> > 
> > Provide simple fix by substituting the variable name with its
> > corresponding upgrade.
> > 
> > Signed-off-by: Aditya Srivastava <yashsri421 at gmail.com>
> > ---
> >  scripts/checkpatch.pl | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index a0107ed257d1..5fd34a862522 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3266,8 +3266,13 @@ sub process {
> >  				'EXTRA_LDFLAGS' =>  'ldflags-y',
> >  			};
> >  
> > -			WARN("DEPRECATED_VARIABLE",
> > -			     "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag});
> > +			if($replacement->{$flag}) {
> > +				if (WARN("DEPRECATED_VARIABLE",
> > +					 "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) &&
> > +				    $fix) {
> > +					$fixed[$fixlinenr] =~ s/$flag/$replacement->{$flag}/;
> > +				}
> > +			}
> >  		}
> >  
> >  # check for DT compatible documentation
> > 
> 
> For this warning, I couldn't find any warning in the past. So, I
> generated a patch for myself, for testing:
>

Well, but if we cannot find any warning in the past, I do not see the 
value of a fix. Who knows how developers would misuse the variable and if 
the simple replacement does not just break everything and makes things 
worse?

How about checking if documentation on this situation is available in the 
kernel documentation?

If so, maybe there are some hints that we could point out here for this 
rule?
I believe for some checkpatch rules it might be worth to point to more 
extensive documentation in the kernel documentation, so developers can 
read a bit more about specific rules.

And if not available, how about understanding the motivation and intent of 
this rule here and providing suitable documentation. That is a much better 
fix than just adding something to checkpatch.pl where we do not know if it 
is ever triggered and where we do not know if the fix makes the situation 
worse.

Lukas
 
> From 1bab609eae606a0b89aa6c752d07b0e64488a6e2 Mon Sep 17 00:00:00 2001
> From: Aditya Srivastava <yashsri421 at gmail.com>
> Date: Sun, 1 Nov 2020 20:12:44 +0530
> Subject: [PATCH] makefile: testing
> 
> random description, random content
> 
> Signed-off-by: Aditya Srivastava <yashsri421 at gmail.com>
> ---
>  scripts/Makefile.lib | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 94133708889d..c9f50e2d9901 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -4,6 +4,7 @@ asflags-y  += $(EXTRA_AFLAGS)
>  ccflags-y  += $(EXTRA_CFLAGS)
>  cppflags-y += $(EXTRA_CPPFLAGS)
>  ldflags-y  += $(EXTRA_LDFLAGS)
> +EXTRA_LDFLAGS  = ldflags-y
>  ifneq ($(always),)
>  $(warning 'always' is deprecated. Please use 'always-y' instead)
>  always-y   += $(always)
> -- 
> 2.17.1
> 
> Running checkpatch.pl on it gives the warning:
> 
> WARNING: Use of EXTRA_LDFLAGS is deprecated, please use `ldflags-y
> instead.
> #21: FILE: scripts/Makefile.lib:7:
> +EXTRA_LDFLAGS  = cppflags-y
> 
> On executing it with --fix produces this result:
> 
> From 1bab609eae606a0b89aa6c752d07b0e64488a6e2 Mon Sep 17 00:00:00 2001
> From: Aditya Srivastava <yashsri421 at gmail.com>
> Date: Sun, 1 Nov 2020 20:12:44 +0530
> Subject: [PATCH] makefile: testing
> 
> random description, random content
> 
> Signed-off-by: Aditya Srivastava <yashsri421 at gmail.com>
> ---
>  scripts/Makefile.lib | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 94133708889d..c9f50e2d9901 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -4,6 +4,7 @@ asflags-y  += $(EXTRA_AFLAGS)
>  ccflags-y  += $(EXTRA_CFLAGS)
>  cppflags-y += $(EXTRA_CPPFLAGS)
>  ldflags-y  += $(EXTRA_LDFLAGS)
> +ldflags-y  = cppflags-y
>  ifneq ($(always),)
>  $(warning 'always' is deprecated. Please use 'always-y' instead)
>  always-y   += $(always)
> -- 
> 2.17.1
> 
> 
> Here, the variable 'EXTRA_LDFLAGS' gets replaced with 'ldflags-y', as
> was suggested in the message.
> 
> 
> Thanks
> Aditya
> 


More information about the Linux-kernel-mentees mailing list