diff mbox series

cmd: setexpr: fix no matching string in gsub return empty value

Message ID 20240202143657.3883166-1-massimiliano.minella@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series cmd: setexpr: fix no matching string in gsub return empty value | expand

Commit Message

Massimiliano Minella Feb. 2, 2024, 2:36 p.m. UTC
From: Massimiliano Minella <massimiliano.minella@se.com>

In gsub, when the destination string is empty, the string 't' is
provided and the regular expression doesn't match, then the final result
is an empty string.

Example:

=> echo ${foo}

=> setenv foo
=> setexpr foo gsub e a bar
=> echo ${foo}

=>

The variable ${foo} should contain "bar" and the lack of match shouldn't
be considered an error.

This patch fixes the erroneous behavior by removing the return
statement and breaking out of the loop in case of lack of match.

Also add a test for the no match case.

Signed-off-by: Massimiliano Minella <massimiliano.minella@se.com>
---
 cmd/setexpr.c      |  6 ++----
 test/cmd/setexpr.c | 10 ++++++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

Comments

Lukas Funke Feb. 5, 2024, 2:28 p.m. UTC | #1
Hi Massimiliano,

On 02.02.2024 15:36, Massimiliano Minella wrote:
> From: Massimiliano Minella <massimiliano.minella@se.com>
> 
> In gsub, when the destination string is empty, the string 't' is
> provided and the regular expression doesn't match, then the final result
> is an empty string.
> 
> Example:
> 
> => echo ${foo}
> 
> => setenv foo
> => setexpr foo gsub e a bar
> => echo ${foo}
> 
> =>
> 
> The variable ${foo} should contain "bar" and the lack of match shouldn't
> be considered an error.
> 
> This patch fixes the erroneous behavior by removing the return
> statement and breaking out of the loop in case of lack of match.
> 
> Also add a test for the no match case.
> 
> Signed-off-by: Massimiliano Minella <massimiliano.minella@se.com>
> ---
>   cmd/setexpr.c      |  6 ++----
>   test/cmd/setexpr.c | 10 ++++++++++
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/setexpr.c b/cmd/setexpr.c
> index 233471f6cb..6a7c0c914b 100644
> --- a/cmd/setexpr.c
> +++ b/cmd/setexpr.c
> @@ -216,14 +216,12 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
>   		if (res == 0) {
>   			if (loop == 0) {
>   				debug("%s: No match\n", data);
> -				return 1;
>   			} else {
> -				break;
> +				debug("## MATCH ## %s\n", data);
>   			}
> +			break;
>   		}
>   
> -		debug("## MATCH ## %s\n", data);
> -
>   		if (!s)
>   			return 1;
>   
> diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c
> index 312593e1e3..ee329e94b8 100644
> --- a/test/cmd/setexpr.c
> +++ b/test/cmd/setexpr.c
> @@ -179,6 +179,16 @@ static int setexpr_test_regex(struct unit_test_state *uts)
>   	val = env_get("mary");
>   	ut_asserteq_str("this is a test", val);
>   
> +	/* No match */
> +	ut_assertok(run_command("setenv fred 'this is a test'", 0));
> +	ut_assertok(run_command("setenv mary ''", 0));
> +	ut_assertok(run_command("setexpr fred gsub us is \"${fred}\"", 0));
> +	ut_assertok(run_command("setexpr mary gsub us is \"${fred}\"", 0));
> +	val = env_get("fred");
> +	ut_asserteq_str("this is a test", val);
> +	val = env_get("mary");
> +	ut_asserteq_str("this is a test", val);
> +
>   	unmap_sysmem(buf);
>   
>   	return 0;

Tested-by: Lukas Funke <lukas.funke-oss@weidmueller.com>

lgtm.

Could you maybe also update the documentation in 
'doc/usage/cmd/setexpr.rst' in order to document this behaviour?

btw: I removed Roland from the address list, since his mail will bounce.

Best regards,
Lukas
diff mbox series

Patch

diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index 233471f6cb..6a7c0c914b 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -216,14 +216,12 @@  int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
 		if (res == 0) {
 			if (loop == 0) {
 				debug("%s: No match\n", data);
-				return 1;
 			} else {
-				break;
+				debug("## MATCH ## %s\n", data);
 			}
+			break;
 		}
 
-		debug("## MATCH ## %s\n", data);
-
 		if (!s)
 			return 1;
 
diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c
index 312593e1e3..ee329e94b8 100644
--- a/test/cmd/setexpr.c
+++ b/test/cmd/setexpr.c
@@ -179,6 +179,16 @@  static int setexpr_test_regex(struct unit_test_state *uts)
 	val = env_get("mary");
 	ut_asserteq_str("this is a test", val);
 
+	/* No match */
+	ut_assertok(run_command("setenv fred 'this is a test'", 0));
+	ut_assertok(run_command("setenv mary ''", 0));
+	ut_assertok(run_command("setexpr fred gsub us is \"${fred}\"", 0));
+	ut_assertok(run_command("setexpr mary gsub us is \"${fred}\"", 0));
+	val = env_get("fred");
+	ut_asserteq_str("this is a test", val);
+	val = env_get("mary");
+	ut_asserteq_str("this is a test", val);
+
 	unmap_sysmem(buf);
 
 	return 0;