Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753961AbcDVOTS (ORCPT <rfc822;w@1wt.eu>);
	Fri, 22 Apr 2016 10:19:18 -0400
Received: from mailgw02.mediatek.com ([210.61.82.184]:6798 "EHLO
	mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org
	with ESMTP id S1752031AbcDVOTR (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 22 Apr 2016 10:19:17 -0400
Message-ID: <1461334749.16530.30.camel@mtksdaap41>
Subject: Re: [PATCH 1/3] checkpatch: add Kconfig 'default n' test
From: Yingjoe Chen <yingjoe.chen@mediatek.com>
To: Joe Perches <joe@perches.com>
CC: Andy Whitcroft <apw@canonical.com>, <linux-kernel@vger.kernel.org>,
        <srv_heupstream@mediatek.com>
Date: Fri, 22 Apr 2016 22:19:09 +0800
In-Reply-To: <1461259011.1918.23.camel@perches.com>
References: <1461245285-14918-1-git-send-email-yingjoe.chen@mediatek.com>
	 <1461259011.1918.23.camel@perches.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.2.3-0ubuntu6 
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MTK: N
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1571
Lines: 59

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