Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1385215

Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test

From Yingjoe Chen <yingjoe.chen@mediatek.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test
Date 2016-04-22 16:20 +0200
Message-ID <rqK65-3fM-11@gated-at.bofh.it> (permalink)
References <rqmQb-1lx-33@gated-at.bofh.it> <rqqqK-4ao-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, 2016-04-21 at 10:16 -0700, Joe Perches wrote:
> On Thu, 2016-04-21 at 21:28 +0800, Yingjoe Chen wrote:
> > If a Kconfig config option doesn't specify 'default', the default
> > will be n. Adding 'default n' is unnecessary.
> > 
> > Add a test to warn about this.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -2600,6 +2600,13 @@ sub process {
> >  			     "Use of boolean is deprecated, please use bool instead.\n" . $herecurr);
> >  		}
> >  
> > +# discourage the use of default n
> > +		if ($realfile =~ /Kconfig/ &&
> > +		    $line =~ /^\+\s*\bdefault\b\s*n\s*$/) {
> 
> maybe line =~ /^\+\s*default\s+n/i


This might match

	default NIOS2

also this won't match lines with comment.
Will change to  $line =~ /^\+\s*default\s*n\s*(#.*|$)/i


> Also, there are some oddities like:
> 
> arch/mips/cavium-octeon/Kconfig:        default "n"


I think this belong to another (new) test: If an option is bool or
tristate, the default value shouldn't be " quoted.


> and a dozen or so uses like
> 
> 	default n if <foo>
> 

I'm not sure about this. With if, it did provide more information.
According to Documentation/kbuild/kconfig-language.txt:
" A config option can have any number of default values. If multiple
  default values are visible, only the first defined one is active."
So the following example will be different when the first one is
removed.

	default n if <foo>
	default y if <bar>

of course we could do the following, but one could argue the above one
is more readable.

	default y if <bar> && ! <foo>

Joe.C

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH 1/3] checkpatch: add Kconfig 'default n' test Yingjoe Chen <yingjoe.chen@mediatek.com> - 2016-04-21 15:30 +0200
  [PATCH 2/3] checkpatch: testing more config for Kconfig help text Yingjoe Chen <yingjoe.chen@mediatek.com> - 2016-04-21 15:30 +0200
    Re: [PATCH 2/3] checkpatch: testing more config for Kconfig help  text Joe Perches <joe@perches.com> - 2016-04-21 19:10 +0200
      Re: [PATCH 2/3] checkpatch: testing more config for Kconfig help  text Yingjoe Chen <yingjoe.chen@mediatek.com> - 2016-04-22 15:40 +0200
  Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test Joe Perches <joe@perches.com> - 2016-04-21 19:20 +0200
    Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test Paul Bolle <pebolle@tiscali.nl> - 2016-04-21 20:30 +0200
    Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test Yingjoe Chen <yingjoe.chen@mediatek.com> - 2016-04-22 16:20 +0200

csiph-web