[v2,01/14] devtools: fix forbidden token check with multiple files

Message ID 20240912082643.1532679-2-david.marchand@redhat.com (mailing list archive)
State Accepted
Delegated to: David Marchand
Headers
Series Use RTE_LOG_LINE in drivers |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

David Marchand Sept. 12, 2024, 8:26 a.m. UTC
If a patch contains multiple files, it is possible to pass through the
check because the count of token mentions is not reset between file
evaluation.

Example with a fake patch:

  $ cat toto.patch
  --- a/drivers/plop1
  +++ b/drivers/plop1
  - RTE_LOG(
  - RTE_LOG(
  + RTE_LOG(
  --- a/drivers/plop2
  +++ b/drivers/plop2
  + RTE_LOG(

  $ awk -v FOLDERS='drivers' -v EXPRESSIONS='RTE_LOG\\('
    -v MESSAGE='Prefer RTE_LOG_LINE'
    -f devtools/check-forbidden-tokens.awk toto.patch

Besides, the expressions[] array is not used since commit
b467d38284b1 ("devtools: add explicit warnings for forbidden tokens").

Fixes: 42f4d724ec27 ("devtools: move awk script ckecking forbidden tokens")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 devtools/check-forbidden-tokens.awk | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
  

Comments

Thomas Monjalon Sept. 12, 2024, 1:54 p.m. UTC | #1
12/09/2024 10:26, David Marchand:
> --- a/devtools/check-forbidden-tokens.awk
> +++ b/devtools/check-forbidden-tokens.awk
> @@ -32,14 +32,11 @@ BEGIN {
>  		for (i in deny_expr) {
>  			forbidden_added = "^\\+.*" deny_expr[i];
>  			forbidden_removed="^-.*" deny_expr[i];
> -			current = expressions[deny_expr[i]]
>  			if ($0 ~ forbidden_added) {
> -				count = count + 1;
> -				expressions[deny_expr[i]] = current + 1
> +				count = count + 1
>  			}
>  			if ($0 ~ forbidden_removed) {
> -				count = count - 1;
> -				expressions[deny_expr[i]] = current - 1
> +				count = count - 1
>  			}
>  		}
>  	}
> @@ -55,6 +52,7 @@ BEGIN {


While at it, you could insert a missing space in "previous filehad new additions"
few lines above.

>  	if (count > 0) {
>  		exit;
>  	}
> +	count = 0

I'm not an awk expert, but it looks simple enough.
Thank you

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  

Patch

diff --git a/devtools/check-forbidden-tokens.awk b/devtools/check-forbidden-tokens.awk
index 90d3a64f8e..807dad7f48 100755
--- a/devtools/check-forbidden-tokens.awk
+++ b/devtools/check-forbidden-tokens.awk
@@ -32,14 +32,11 @@  BEGIN {
 		for (i in deny_expr) {
 			forbidden_added = "^\\+.*" deny_expr[i];
 			forbidden_removed="^-.*" deny_expr[i];
-			current = expressions[deny_expr[i]]
 			if ($0 ~ forbidden_added) {
-				count = count + 1;
-				expressions[deny_expr[i]] = current + 1
+				count = count + 1
 			}
 			if ($0 ~ forbidden_removed) {
-				count = count - 1;
-				expressions[deny_expr[i]] = current - 1
+				count = count - 1
 			}
 		}
 	}
@@ -55,6 +52,7 @@  BEGIN {
 	if (count > 0) {
 		exit;
 	}
+	count = 0
 	for (i in deny_folders) {
 		re = "^\\+\\+\\+ b/" deny_folders[i];
 		if ($0 ~ re) {