From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 789645F19 for ; Fri, 19 Oct 2018 22:33:16 +0200 (CEST) Received: from [107.15.85.130] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1gDLYi-0000a5-L9; Thu, 18 Oct 2018 23:34:56 -0400 Date: Fri, 19 Oct 2018 16:32:52 -0400 From: Neil Horman To: "Liang, Ma" Cc: Thomas Monjalon , dev@dpdk.org Message-ID: <20181019203252.GB30254@hmswarspite.think-freely.org> References: <1538666296-25742-1-git-send-email-liang.j.ma@intel.com> <5107033.R7BcetJnk1@xps> <20181019113815.GA30254@hmswarspite.think-freely.org> <20181019130951.GC1349@sivswdev01.ir.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181019130951.GC1349@sivswdev01.ir.intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Score: 0.3 (/) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Oct 2018 20:33:16 -0000 On Fri, Oct 19, 2018 at 02:09:51PM +0100, Liang, Ma wrote: > Hi Neil, > there are two things here. > 1. This issue give me negative report when I run checkpatch. > So, I hope we can disable symbol check before we fix it. > You're welcome to disable the check locally I suppose, but I'll not agree to a patch that does that. I understand you have a false negative report, but the right thing to do is fix it. I appreciate you wanting to do that, but I don't think what you have below is the right answer. > 2. How to fix that > I still don't understand why we need match section name start with '+'. > The section name should start with 'A-Z' only for my opinion. > We don't, thats not what I said. Your patch changes the match rule to enforce not having a + or - in the lead of the line defining the section. But, as the patch you triggered on shows, that might be the line which defines what the section is, so instead of ignoring those lines (which this patch does), what I suggests was keeping the match line exactly as is, and add a line in the actions to strip the + or - out, something like this: /^.*{/ { ... sec=$(NF-1);sec=`echo $sec | sed -e"s/[+-]//g"; in_sec=1 } that way you get section names when they're changing, and still can remove the leading cruft Regards Neil > Liang > > On 19 Oct 07:38, Neil Horman wrote: > > On Fri, Oct 19, 2018 at 12:48:57PM +0200, Thomas Monjalon wrote: > > > Hi Neil, > > > > > > Are you OK with this patch? > > > > > > > > > 04/10/2018 17:18, Liang Ma: > > > > There is a issue inside check-symbol-changes.sh awk script. > > > > When the script try to parse the section name from patch, > > > > The script put char "+" into the section name. > > > > > > > > Signed-off-by: Liang Ma > > > > --- > > > > devtools/check-symbol-change.sh | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh > > > > index c0d2a6d..4a0f4d8 100755 > > > > --- a/devtools/check-symbol-change.sh > > > > +++ b/devtools/check-symbol-change.sh > > > > @@ -30,7 +30,7 @@ build_map_changes() > > > > # the rest of the line with the + and { symbols remvoed. > > > > # Triggering this rule sets in_sec to 1, which actives the > > > > # symbol rule below > > > > - /^.*{/ { > > > > + /^[^-+]*{/ { > > > > if (in_map == 1) { > > > > sec=$(NF-1); in_sec=1; > > > > } > > > > > > > > > > > > > > > > I don't think so. The section name might get a + in front of it on lines like > > this: > > > > +EXPERIMENTAL { > > > > But the above change just skips matching on it, which I don't think is what we > > want. The right fix I think would be to match on it, then strip the leading > > plus with a sed operation in the body of the match rule. > > > > Neil > > >