From patchwork Tue Aug 27 13:55:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Miles Chen X-Patchwork-Id: 11117045 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 213A813B1 for ; Tue, 27 Aug 2019 13:56:18 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F244420828 for ; Tue, 27 Aug 2019 13:56:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Q4JYHmpe" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F244420828 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:Subject:To :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=0VpGJmWsMOMX/QJWL3jH27zCCazjuUAoMItRoxb3vZo=; b=Q4JYHmpehpvvvS A9trWluRBcoEAxplbqLkWxhZHkJDM6//m6ojQoayU5qFW4nfd8+Dfav9R3xlfZWJYKAGMcE+5zVQn Mk0ixZvfXJcWrYw4+C5To3YtzL8XCjH9Oc9qinDoYX40UsO25lXqlPZt+jCVhJmPdXUeAS63Rljl2 iluZN4t9kKNrBv6xkjaBl0msueBGon3CrkmX2kgVjfB7eeHOyhW9hSf92oN4j4ynP2zCLnDh4lrgd kymcMuqtoVUlV3bV9CgbkPNR6/8sFGXAjzP+OemY1QSDH4X3qbRCFRrpOQPqcT/iXTrxzIyN5Dk48 ArdeGNvJ0of9cp0CcvEQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1i2bxJ-0001nN-DR; Tue, 27 Aug 2019 13:56:17 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1i2bxB-0001br-PC for linux-mediatek@lists.infradead.org; Tue, 27 Aug 2019 13:56:11 +0000 X-UUID: 52022c8de15340d4b5ac77caf27b090e-20190827 X-UUID: 52022c8de15340d4b5ac77caf27b090e-20190827 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 380526297; Tue, 27 Aug 2019 05:56:12 -0800 Received: from MTKMBS01N2.mediatek.inc (172.21.101.79) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 27 Aug 2019 06:56:10 -0700 Received: from mtkcas08.mediatek.inc (172.21.101.126) by mtkmbs01n2.mediatek.inc (172.21.101.79) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 27 Aug 2019 21:55:56 +0800 Received: from mtksdccf07.mediatek.inc (172.21.84.99) by mtkcas08.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Tue, 27 Aug 2019 21:55:56 +0800 From: Miles Chen To: Andy Whitcroft , Joe Perches Subject: [RESEND PATCH v4] checkpatch: add several Kconfig default value tests Date: Tue, 27 Aug 2019 21:55:47 +0800 Message-ID: <20190827135547.23131-1-miles.chen@mediatek.com> X-Mailer: git-send-email 2.18.0 MIME-Version: 1.0 X-TM-SNTS-SMTP: 5C6B16DABEAC2A22C971A115B57E60A13784C980BAD27267ED660BEB496236D42000:8 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190827_065609_922406_60DC3170 X-CRM114-Status: GOOD ( 15.36 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [216.200.240.184 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 UNPARSEABLE_RELAY Informational: message has unparseable relay lines X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Yingjoe Chen , Miles Chen , linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, wsd_upstream@mediatek.com Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org This change adds 3 Kconfig default value tests. 1. Discourage default n cases: e.g., default n 2. Discourage default "[ynm]" cases: e.g., arch/powerpc/Kconfig: default "y" if PPC_POWERNV arch/powerpc/Kconfig: default "y" if PPC_POWERNV arch/powerpc/Kconfig: default "n" drivers/auxdisplay/Kconfig: default "n" drivers/crypto/Kconfig: default "m" drivers/rapidio/devices/Kconfig: default "n" 3. Discourage default EXPERT or default !EXPERT cases: e.g., drivers/hid/Kconfig: default !EXPERT tested cases: default m default n if ALPHA_EV5 || ALPHA_EV56 || (ALPHA_EV4 && !ALPHA_LCA) default y if ALPHA_QEMU default n if PPC_POWERNV default n default EXPERT default !EXPERT default "m" default "n" default "y" if EXPERT default "y" if PPC_POWERNV test result: WARNING: 'default n' is the default value, no need to write it explicitly + default n WARNING: Avoid using default EXPERT + default EXPERT WARNING: Avoid using default EXPERT + default !EXPERT WARNING: Use 'default m' not 'default "m"' + default "m" WARNING: Use 'default n' not 'default "n"' + default "n" WARNING: Use 'default y' not 'default "y"' + default "y" if EXPERT WARNING: Use 'default y' not 'default "y"' + default "y" if PPC_POWERNV test --fix capability: default n => delete line default "m" => default m default "n" => default n default "y" if EXPERT => default y if EXPERT default "y" if PPC_POWERNV => default y if PPC_POWERNV default !EXPERT => no change default EXPERT => no change Change since v1: discourage default n$ discourage default "[ynm]" discourage default \!?EXPERT Change since v2: (Joe has provided the whole patch and I just post it) test Kconfig in a single block print precise message such as 'default "m"', not 'default "[ynm]"' provide --fix capability Change since v3: resend patch, no change Cc: Joe Perches Cc: Yingjoe Chen Signed-off-by: Miles Chen --- scripts/checkpatch.pl | 139 ++++++++++++++++++++++++++---------------- 1 file changed, 85 insertions(+), 54 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 93a7edfe0f05..b080eea68cf6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2932,60 +2932,98 @@ sub process { "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet) } -# check for Kconfig help text having a real description -# Only applies when adding the entry originally, after that we do not have -# sufficient context to determine whether it is indeed long enough. - if ($realfile =~ /Kconfig/ && - # 'choice' is usually the last thing on the line (though - # Kconfig supports named choices), so use a word boundary - # (\b) rather than a whitespace character (\s) - $line =~ /^\+\s*(?:config|menuconfig|choice)\b/) { - my $length = 0; - my $cnt = $realcnt; - my $ln = $linenr + 1; - my $f; - my $is_start = 0; - my $is_end = 0; - for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) { - $f = $lines[$ln - 1]; - $cnt-- if ($lines[$ln - 1] !~ /^-/); - $is_end = $lines[$ln - 1] =~ /^\+/; - - next if ($f =~ /^-/); - last if (!$file && $f =~ /^\@\@/); - - if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { - $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) { - if ($lines[$ln - 1] =~ "---help---") { - WARN("CONFIG_DESCRIPTION", - "prefer 'help' over '---help---' for new help texts\n" . $herecurr); +# Kconfig tests + if ($realfile =~ /Kconfig/) { + # check for Kconfig help text having a real description + # Only applies when adding the entry originally, after + # that we do not have sufficient context to determine + # whether it is indeed long enough. + # 'choice' is usually the last thing on the line (though + # Kconfig supports named choices), so use a word + # boundary (\b) rather than a whitespace character (\s) + if ($line =~ /^\+\s*(?:config|menuconfig|choice)\b/) { + my $length = 0; + my $cnt = $realcnt; + my $ln = $linenr + 1; + my $f; + my $is_start = 0; + my $is_end = 0; + for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) { + $f = $lines[$ln - 1]; + $cnt-- if ($lines[$ln - 1] !~ /^-/); + $is_end = $lines[$ln - 1] =~ /^\+/; + + next if ($f =~ /^-/); + last if (!$file && $f =~ /^\@\@/); + + if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { + $is_start = 1; + } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) { + if ($lines[$ln - 1] =~ "---help---") { + WARN("CONFIG_DESCRIPTION", + "prefer 'help' over '---help---' for new help texts\n" . $herecurr); + } + $length = -1; + } + + $f =~ s/^.//; + $f =~ s/#.*//; + $f =~ s/^\s+//; + next if ($f =~ /^$/); + + # This only checks context lines in the patch + # and so hopefully shouldn't trigger false + # positives, even though some of these are + # common words in help texts + if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice| + if|endif|menu|endmenu|source)\b/x) { + $is_end = 1; + last; } - $length = -1; + $length++; + } + if ($is_start && $is_end && $length < $min_conf_desc_length) { + WARN("CONFIG_DESCRIPTION", + "please write a paragraph that describes the config symbol fully\n" . $herecurr); } + #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; + } - $f =~ s/^.//; - $f =~ s/#.*//; - $f =~ s/^\s+//; - next if ($f =~ /^$/); - - # This only checks context lines in the patch - # and so hopefully shouldn't trigger false - # positives, even though some of these are - # common words in help texts - if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice| - if|endif|menu|endmenu|source)\b/x) { - $is_end = 1; - last; +# discourage the use of boolean for type definition attributes + if ($line =~ /^\+\s*\bboolean\b/) { + if (WARN("CONFIG_TYPE_BOOLEAN", + "Use of boolean is deprecated, please use bool instead\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bboolean\b/bool/; + } + } + +# Kconfig: discourage redundant 'default n' + if ($line =~ /^\+\s*default\s+n$/) { + if (WARN("CONFIG_DEFAULT_VALUE_STYLE", + "'default n' is the default value, no need to write it explicitly\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); } - $length++; } - if ($is_start && $is_end && $length < $min_conf_desc_length) { - WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + +# Kconfig: discourage quoted defaults: use default [ynm], not default "[ynm]" + if ($rawline =~ /^\+\s*default\s+"([ynm])"/) { + if (WARN("CONFIG_DEFAULT_VALUE_STYLE", + "Use 'default $1' not 'default \"$1\"'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b(default\s+)"(.)"/$1$2/; + } + } + +# Kconfig: discourage using default EXPERT or !EXPERT + if ($line =~ /^\+\s*default\s+\!?\s*EXPERT\b/) { + WARN("CONFIG_DEFAULT_VALUE_STYLE", + "Avoid using default EXPERT\n" . $herecurr); } - #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } +# End of Kconfig tests + # check for MAINTAINERS entries that don't have the right form if ($realfile =~ /^MAINTAINERS$/ && @@ -2998,13 +3036,6 @@ sub process { } } -# discourage the use of boolean for type definition attributes of Kconfig options - if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*\bboolean\b/) { - WARN("CONFIG_TYPE_BOOLEAN", - "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); - } - if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { my $flag = $1;