* [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue @ 2018-10-04 15:18 Liang Ma 2018-10-19 10:48 ` Thomas Monjalon 0 siblings, 1 reply; 5+ messages in thread From: Liang Ma @ 2018-10-04 15:18 UTC (permalink / raw) To: nhorman; +Cc: dev, 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 <liang.j.ma@intel.com> --- 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; } -- 2.7.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue 2018-10-04 15:18 [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue Liang Ma @ 2018-10-19 10:48 ` Thomas Monjalon 2018-10-19 11:38 ` Neil Horman 0 siblings, 1 reply; 5+ messages in thread From: Thomas Monjalon @ 2018-10-19 10:48 UTC (permalink / raw) To: nhorman; +Cc: dev, Liang Ma 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 <liang.j.ma@intel.com> > --- > 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; > } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue 2018-10-19 10:48 ` Thomas Monjalon @ 2018-10-19 11:38 ` Neil Horman 2018-10-19 13:09 ` Liang, Ma 0 siblings, 1 reply; 5+ messages in thread From: Neil Horman @ 2018-10-19 11:38 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, Liang Ma 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 <liang.j.ma@intel.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue 2018-10-19 11:38 ` Neil Horman @ 2018-10-19 13:09 ` Liang, Ma 2018-10-19 20:32 ` Neil Horman 0 siblings, 1 reply; 5+ messages in thread From: Liang, Ma @ 2018-10-19 13:09 UTC (permalink / raw) To: Neil Horman; +Cc: Thomas Monjalon, dev 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. 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. Regards 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 <liang.j.ma@intel.com> > > > --- > > > 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue 2018-10-19 13:09 ` Liang, Ma @ 2018-10-19 20:32 ` Neil Horman 0 siblings, 0 replies; 5+ messages in thread From: Neil Horman @ 2018-10-19 20:32 UTC (permalink / raw) To: Liang, Ma; +Cc: Thomas Monjalon, dev 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 <liang.j.ma@intel.com> > > > > --- > > > > 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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-19 20:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-04 15:18 [dpdk-dev] [PATCH] devtools/check-symbol-changes.sh: awk script issue Liang Ma 2018-10-19 10:48 ` Thomas Monjalon 2018-10-19 11:38 ` Neil Horman 2018-10-19 13:09 ` Liang, Ma 2018-10-19 20:32 ` Neil Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).