check-symbol-change: relax rule for identifying a section
diff mbox series

Message ID 20180816110801.5378-1-nhorman@tuxdriver.com
State Accepted, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • check-symbol-change: relax rule for identifying a section
Related show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Neil Horman Aug. 16, 2018, 11:08 a.m. UTC
It was reported recently that some patches that add symbols to an
existing EXPERIMENTAL section of a version map file generate errors
because the check-symbol-change script was identifying the section as
"@@" rather than EXPERIMENTAL.  This was fairly clearly due to the fact
that the rule identifying the version section expected the whole section
to be added, rather than having it already exist, with only new symbols
being added to the existing section. This led the match rule to misread
the format of that line and pull the wrong word out of it.

The fix is to relax the rule slightly.  Rather than assume that the
section must exist on a line that was added, allow the section name to
be set by any line that ends in a '{', which should be correct, given
our coding practices.  The section name is then extracted as the next to
the last word on the line ( $(NF-1) ).

Tested by the reporter with good results

Reported-by: nikhil.rao@intel.com
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas@monjalon.net
CC: nikhil.rao@intel.com
---
 devtools/check-symbol-change.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Rao, Nikhil Aug. 17, 2018, 3:03 a.m. UTC | #1
On 8/16/2018 4:38 PM, Neil Horman wrote:
> It was reported recently that some patches that add symbols to an
> existing EXPERIMENTAL section of a version map file generate errors
> because the check-symbol-change script was identifying the section as
> "@@" rather than EXPERIMENTAL.  This was fairly clearly due to the fact
> that the rule identifying the version section expected the whole section
> to be added, rather than having it already exist, with only new symbols
> being added to the existing section. This led the match rule to misread
> the format of that line and pull the wrong word out of it.
> 
> The fix is to relax the rule slightly.  Rather than assume that the
> section must exist on a line that was added, allow the section name to
> be set by any line that ends in a '{', which should be correct, given
> our coding practices.  The section name is then extracted as the next to
> the last word on the line ( $(NF-1) ).
> 
> Tested by the reporter with good results
> 
> Reported-by: nikhil.rao@intel.com
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: thomas@monjalon.net
> CC: nikhil.rao@intel.com
> ---
>   devtools/check-symbol-change.sh | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
> index daaf45e14..cf9cfc745 100755
> --- a/devtools/check-symbol-change.sh
> +++ b/devtools/check-symbol-change.sh
> @@ -25,14 +25,14 @@ build_map_changes()
>   		# supresses the subordonate rules below
>   		/[-+] a\/.*\.^(map)/ {in_map=0}
>   
> -		# Triggering this rule, which starts a line with a + and ends it
> +		# Triggering this rule, which starts a line and ends it
>   		# with a { identifies a versioned section.  The section name is
>   		# the rest of the line with the + and { symbols remvoed.
>   		# Triggering this rule sets in_sec to 1, which actives the
>   		# symbol rule below
> -		/+.*{/ {gsub("+","");
> +		/^.*{/ {
>   			if (in_map == 1) {
> -				sec=$1; in_sec=1;
> +				sec=$(NF-1); in_sec=1;
>   			}
>   		}
>   
> 
Tested-by: Nikhil Rao <nikhil.rao@intel.com>
Thomas Monjalon Aug. 21, 2018, 9:03 a.m. UTC | #2
17/08/2018 05:03, Rao, Nikhil:
> On 8/16/2018 4:38 PM, Neil Horman wrote:
> > It was reported recently that some patches that add symbols to an
> > existing EXPERIMENTAL section of a version map file generate errors
> > because the check-symbol-change script was identifying the section as
> > "@@" rather than EXPERIMENTAL.  This was fairly clearly due to the fact
> > that the rule identifying the version section expected the whole section
> > to be added, rather than having it already exist, with only new symbols
> > being added to the existing section. This led the match rule to misread
> > the format of that line and pull the wrong word out of it.
> > 
> > The fix is to relax the rule slightly.  Rather than assume that the
> > section must exist on a line that was added, allow the section name to
> > be set by any line that ends in a '{', which should be correct, given
> > our coding practices.  The section name is then extracted as the next to
> > the last word on the line ( $(NF-1) ).
> > 
> > Tested by the reporter with good results
> > 
> > Reported-by: nikhil.rao@intel.com
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> Tested-by: Nikhil Rao <nikhil.rao@intel.com>

Applied, thanks

Patch
diff mbox series

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index daaf45e14..cf9cfc745 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -25,14 +25,14 @@  build_map_changes()
 		# supresses the subordonate rules below
 		/[-+] a\/.*\.^(map)/ {in_map=0}
 
-		# Triggering this rule, which starts a line with a + and ends it
+		# Triggering this rule, which starts a line and ends it
 		# with a { identifies a versioned section.  The section name is
 		# the rest of the line with the + and { symbols remvoed.
 		# Triggering this rule sets in_sec to 1, which actives the
 		# symbol rule below
-		/+.*{/ {gsub("+","");
+		/^.*{/ {
 			if (in_map == 1) {
-				sec=$1; in_sec=1;
+				sec=$(NF-1); in_sec=1;
 			}
 		}