[PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[Jeremy Sowden]

The flow-control of `bm_find` is very deeply nested with a conditional
comparing a ternary expression against the pattern inside a for-loop
inside a while-loop inside a for-loop.

Move the inner for-loop into a helper function to reduce the amount of
indentation and make the code easier to read.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 lib/ts_bm.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/lib/ts_bm.c b/lib/ts_bm.c
index 1f2234221dd1..d74fdb87d269 100644
--- a/lib/ts_bm.c
+++ b/lib/ts_bm.c
@@ -55,6 +55,24 @@ struct ts_bm
 	unsigned int	good_shift[];
 };
 
+static bool patmtch(const u8 *pattern, const u8 *text, unsigned int patlen,
+		    bool icase)
+{
+	unsigned int i;
+
+	for (i = 0; i < patlen; i++) {
+		u8 t = *(text-i);
+
+		if (icase)
+			t = toupper(t);
+
+		if (t != *(pattern-i))
+			return false;
+	}
+
+	return true;
+}
+
 static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
 {
 	struct ts_bm *bm = ts_config_priv(conf);
@@ -70,19 +88,17 @@ static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
 			break;
 
 		while (shift < text_len) {
-			DEBUGP("Searching in position %d (%c)\n", 
-				shift, text[shift]);
-			for (i = 0; i < bm->patlen; i++) 
-				if ((icase ? toupper(text[shift-i])
-				    : text[shift-i])
-					!= bm->pattern[bm->patlen-1-i])
-				     goto next;
-
-			/* London calling... */
-			DEBUGP("found!\n");
-			return consumed + (shift-(bm->patlen-1));
-
-next:			bs = bm->bad_shift[text[shift-i]];
+			DEBUGP("Searching in position %d (%c)\n",
+			       shift, text[shift]);
+
+			if (patmtch(&bm->pattern[bm->patlen-1], &text[shift],
+				    bm->patlen, icase)) {
+				/* London calling... */
+				DEBUGP("found!\n");
+				return consumed + (shift-(bm->patlen-1));
+			}
+
+			bs = bm->bad_shift[text[shift-i]];
 
 			/* Now jumping to... */
 			shift = max_t(int, shift-i+bs, shift+bm->good_shift[i]);
-- 
2.39.2

Re: [PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[kernel test robot]

Hi Jeremy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.4-rc7 next-20230620]
[cannot apply to nf-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremy-Sowden/lib-ts_bm-add-helper-to-reduce-indentation-and-improve-readability/20230620-031043
base:   linus/master
patch link:    https://lore.kernel.org/r/20230619190803.1906012-1-jeremy%40azazel.net
patch subject: [PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230620/202306201819.BoQ5IT7Y-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230620/202306201819.BoQ5IT7Y-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306201819.BoQ5IT7Y-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> lib/ts_bm.c:101:34: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
                           bs = bm->bad_shift[text[shift-i]];
                                                         ^
   lib/ts_bm.c:79:16: note: initialize the variable 'i' to silence this warning
           unsigned int i, text_len, consumed = state->offset;
                         ^
                          = 0
   1 warning generated.


vim +/i +101 lib/ts_bm.c

    75	
    76	static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
    77	{
    78		struct ts_bm *bm = ts_config_priv(conf);
    79		unsigned int i, text_len, consumed = state->offset;
    80		const u8 *text;
    81		int shift = bm->patlen - 1, bs;
    82		const u8 icase = conf->flags & TS_IGNORECASE;
    83	
    84		for (;;) {
    85			text_len = conf->get_next_block(consumed, &text, conf, state);
    86	
    87			if (unlikely(text_len == 0))
    88				break;
    89	
    90			while (shift < text_len) {
    91				DEBUGP("Searching in position %d (%c)\n",
    92				       shift, text[shift]);
    93	
    94				if (patmtch(&bm->pattern[bm->patlen-1], &text[shift],
    95					    bm->patlen, icase)) {
    96					/* London calling... */
    97					DEBUGP("found!\n");
    98					return consumed + (shift-(bm->patlen-1));
    99				}
   100	
 > 101				bs = bm->bad_shift[text[shift-i]];
   102	
   103				/* Now jumping to... */
   104				shift = max_t(int, shift-i+bs, shift+bm->good_shift[i]);
   105			}
   106			consumed += text_len;
   107		}
   108	
   109		return UINT_MAX;
   110	}
   111	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]

On 2023-06-20, at 18:42:43 +0800, kernel test robot wrote:
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.4-rc7 next-20230620]
> [cannot apply to nf-next/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jeremy-Sowden/lib-ts_bm-add-helper-to-reduce-indentation-and-improve-readability/20230620-031043
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20230619190803.1906012-1-jeremy%40azazel.net
> patch subject: [PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230620/202306201819.BoQ5IT7Y-lkp@intel.com/config)
> compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> reproduce: (https://download.01.org/0day-ci/archive/20230620/202306201819.BoQ5IT7Y-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202306201819.BoQ5IT7Y-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> lib/ts_bm.c:101:34: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
>                            bs = bm->bad_shift[text[shift-i]];
>                                                          ^
>    lib/ts_bm.c:79:16: note: initialize the variable 'i' to silence this warning
>            unsigned int i, text_len, consumed = state->offset;
>                          ^
>                           = 0
>    1 warning generated.

Whoops.  Will fix and send v2.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[Jeremy Sowden]

The flow-control of `bm_find` is very deeply nested with a conditional
comparing a ternary expression against the pattern inside a for-loop
inside a while-loop inside a for-loop.

Move the inner for-loop into a helper function to reduce the amount of
indentation and make the code easier to read.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 lib/ts_bm.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/lib/ts_bm.c b/lib/ts_bm.c
index 1f2234221dd1..d74fdb87d269 100644
--- a/lib/ts_bm.c
+++ b/lib/ts_bm.c
@@ -55,6 +55,24 @@ struct ts_bm
 	unsigned int	good_shift[];
 };
 
+static bool patmtch(const u8 *pattern, const u8 *text, unsigned int patlen,
+		    bool icase)
+{
+	unsigned int i;
+
+	for (i = 0; i < patlen; i++) {
+		u8 t = *(text-i);
+
+		if (icase)
+			t = toupper(t);
+
+		if (t != *(pattern-i))
+			return false;
+	}
+
+	return true;
+}
+
 static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
 {
 	struct ts_bm *bm = ts_config_priv(conf);
@@ -70,19 +88,17 @@ static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
 			break;
 
 		while (shift < text_len) {
-			DEBUGP("Searching in position %d (%c)\n", 
-				shift, text[shift]);
-			for (i = 0; i < bm->patlen; i++) 
-				if ((icase ? toupper(text[shift-i])
-				    : text[shift-i])
-					!= bm->pattern[bm->patlen-1-i])
-				     goto next;
-
-			/* London calling... */
-			DEBUGP("found!\n");
-			return consumed + (shift-(bm->patlen-1));
-
-next:			bs = bm->bad_shift[text[shift-i]];
+			DEBUGP("Searching in position %d (%c)\n",
+			       shift, text[shift]);
+
+			if (patmtch(&bm->pattern[bm->patlen-1], &text[shift],
+				    bm->patlen, icase)) {
+				/* London calling... */
+				DEBUGP("found!\n");
+				return consumed + (shift-(bm->patlen-1));
+			}
+
+			bs = bm->bad_shift[text[shift-i]];
 
 			/* Now jumping to... */
 			shift = max_t(int, shift-i+bs, shift+bm->good_shift[i]);
-- 
2.39.2

Re: [PATCH nf-next] lib/ts_bm: add helper to reduce indentation and improve readability

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2316 bytes --]

On 2023-06-20, at 19:09:25 +0100, Jeremy Sowden wrote:
> The flow-control of `bm_find` is very deeply nested with a conditional
> comparing a ternary expression against the pattern inside a for-loop
> inside a while-loop inside a for-loop.
> 
> Move the inner for-loop into a helper function to reduce the amount of
> indentation and make the code easier to read.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  lib/ts_bm.c | 42 +++++++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 13 deletions(-)

Sent the wrong version.  Apologies.

J.

> diff --git a/lib/ts_bm.c b/lib/ts_bm.c
> index 1f2234221dd1..d74fdb87d269 100644
> --- a/lib/ts_bm.c
> +++ b/lib/ts_bm.c
> @@ -55,6 +55,24 @@ struct ts_bm
>  	unsigned int	good_shift[];
>  };
>  
> +static bool patmtch(const u8 *pattern, const u8 *text, unsigned int patlen,
> +		    bool icase)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < patlen; i++) {
> +		u8 t = *(text-i);
> +
> +		if (icase)
> +			t = toupper(t);
> +
> +		if (t != *(pattern-i))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
>  {
>  	struct ts_bm *bm = ts_config_priv(conf);
> @@ -70,19 +88,17 @@ static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
>  			break;
>  
>  		while (shift < text_len) {
> -			DEBUGP("Searching in position %d (%c)\n", 
> -				shift, text[shift]);
> -			for (i = 0; i < bm->patlen; i++) 
> -				if ((icase ? toupper(text[shift-i])
> -				    : text[shift-i])
> -					!= bm->pattern[bm->patlen-1-i])
> -				     goto next;
> -
> -			/* London calling... */
> -			DEBUGP("found!\n");
> -			return consumed + (shift-(bm->patlen-1));
> -
> -next:			bs = bm->bad_shift[text[shift-i]];
> +			DEBUGP("Searching in position %d (%c)\n",
> +			       shift, text[shift]);
> +
> +			if (patmtch(&bm->pattern[bm->patlen-1], &text[shift],
> +				    bm->patlen, icase)) {
> +				/* London calling... */
> +				DEBUGP("found!\n");
> +				return consumed + (shift-(bm->patlen-1));
> +			}
> +
> +			bs = bm->bad_shift[text[shift-i]];
>  
>  			/* Now jumping to... */
>  			shift = max_t(int, shift-i+bs, shift+bm->good_shift[i]);
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]