diff mbox series

[libubootenv] Force writing of environment if default is used

Message ID 20201215165025.824324-1-mark.jonas@de.bosch.com
State Accepted
Headers show
Series [libubootenv] Force writing of environment if default is used | expand

Commit Message

Jonas Mark (BT-FIR/ENG1-Grb) Dec. 15, 2020, 4:50 p.m. UTC
From: Wang Xin <xin.wang7@cn.bosch.com>

Before commit ed1a53ec "Dont store to device if no value changes" it was
possible to fill an uninitialized or broken environment using the
command "fw_setenv -f <default-environment-file>". This was considered
to be a feature.

The problem occurs because fw_setenv will now skip writing the
environment in case no environment variables to be changed are passed to
fw_setenv.

This commit will fix this problem by always storing the environment if
loading the environment failed and a default environment was given.

Signed-off-by: Wang Xin <xin.wang7@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
---
 src/fw_printenv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Stefano Babic Dec. 15, 2020, 2:56 p.m. UTC | #1
Hi Mark,

On 15.12.20 17:50, 'Mark Jonas' via swupdate wrote:
> From: Wang Xin <xin.wang7@cn.bosch.com>
> 
> Before commit ed1a53ec "Dont store to device if no value changes" it was
> possible to fill an uninitialized or broken environment using the
> command "fw_setenv -f <default-environment-file>". This was considered
> to be a feature.
> 
> The problem occurs because fw_setenv will now skip writing the
> environment in case no environment variables to be changed are passed to
> fw_setenv.
> 

Ok, it is a hidden feature...

> This commit will fix this problem by always storing the environment if
> loading the environment failed and a default environment was given.
> 
> Signed-off-by: Wang Xin <xin.wang7@cn.bosch.com>
> Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> ---
>  src/fw_printenv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/fw_printenv.c b/src/fw_printenv.c
> index 8b830d5..44ad849 100644
> --- a/src/fw_printenv.c
> +++ b/src/fw_printenv.c
> @@ -66,6 +66,7 @@ int main (int argc, char **argv) {
>  	char *progname;
>  	bool is_setenv = false;
>  	bool noheader = false;
> +	bool default_used = false;
>  
>  	/*
>  	 * As old tool, there is just a tool with symbolic link
> @@ -130,6 +131,7 @@ int main (int argc, char **argv) {
>  			fprintf(stderr, "Cannot read default environment from file\n");
>  			exit (ret);
>  		}
> +		default_used = true;
>  	}
>  
>  	if (!is_setenv) {
> @@ -172,7 +174,7 @@ int main (int argc, char **argv) {
>  			}
>  		}
>  
> -		if (need_store) {
> +		if (need_store || default_used) {
>  			ret = libuboot_env_store(ctx);
>  			if (ret)
>  				fprintf(stderr, "Error storing the env\n");
> 

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/src/fw_printenv.c b/src/fw_printenv.c
index 8b830d5..44ad849 100644
--- a/src/fw_printenv.c
+++ b/src/fw_printenv.c
@@ -66,6 +66,7 @@  int main (int argc, char **argv) {
 	char *progname;
 	bool is_setenv = false;
 	bool noheader = false;
+	bool default_used = false;
 
 	/*
 	 * As old tool, there is just a tool with symbolic link
@@ -130,6 +131,7 @@  int main (int argc, char **argv) {
 			fprintf(stderr, "Cannot read default environment from file\n");
 			exit (ret);
 		}
+		default_used = true;
 	}
 
 	if (!is_setenv) {
@@ -172,7 +174,7 @@  int main (int argc, char **argv) {
 			}
 		}
 
-		if (need_store) {
+		if (need_store || default_used) {
 			ret = libuboot_env_store(ctx);
 			if (ret)
 				fprintf(stderr, "Error storing the env\n");