[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[zqb-all]

Signed-off-by: zqb-all <zhuangqiubin@gmail.com>
---
v2: fix all "env = environment.data" and "env = default_environment"

 tools/env/fw_env.c | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index a5d7595..60742b6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -398,7 +398,14 @@ char *fw_getenv(char *name)
 {
 	char *env, *nxt;
 
-	for (env = environment.data; *env; env = nxt + 1) {
+	for (nxt = environment.data; !(*nxt); ++nxt) {
+		if (nxt >= &environment.data[ENV_SIZE]) {
+			fprintf (stderr, "## Error: "
+				"environment is empty\n");
+			return NULL;
+		}
+	}
+	for (env = nxt; *env; env = nxt + 1) {
 		char *val;
 
 		for (nxt = env; *nxt; ++nxt) {
@@ -423,8 +430,14 @@ char *fw_getenv(char *name)
 char *fw_getdefenv(char *name)
 {
 	char *env, *nxt;
-
-	for (env = default_environment; *env; env = nxt + 1) {
+	for (nxt = default_environment; !(*nxt); ++nxt) {
+		if (nxt >= &default_environment[ENV_SIZE]) {
+			fprintf (stderr, "## Error: "
+				"default environment is empty\n");
+			return NULL;
+		}
+	}
+	for (env = nxt; *env; env = nxt + 1) {
 		char *val;
 
 		for (nxt = env; *nxt; ++nxt) {
@@ -464,7 +477,14 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 
 	if (argc == 0) {	/* Print all env variables  */
 		char *env, *nxt;
-		for (env = environment.data; *env; env = nxt + 1) {
+		for (nxt = environment.data; !(*nxt); ++nxt) {
+			if (nxt >= &environment.data[ENV_SIZE]) {
+				fprintf (stderr, "## Error: "
+					"environment is empty\n");
+				return -1;
+			}
+		}
+		for (env = nxt; *env; env = nxt + 1) {
 			for (nxt = env; *nxt; ++nxt) {
 				if (nxt >= &environment.data[ENV_SIZE]) {
 					fprintf(stderr, "## Error: "
@@ -537,7 +557,14 @@ int fw_env_write(char *name, char *value)
 	/*
 	 * search if variable with this name already exists
 	 */
-	for (nxt = env = environment.data; *env; env = nxt + 1) {
+	for (nxt = environment.data; !(*nxt); ++nxt) {
+		if (nxt >= &environment.data[ENV_SIZE]) {
+			fprintf (stderr, "## Error: "
+				"environment is empty\n");
+			return -1;
+		}
+	}
+	for (env = nxt; *env; env = nxt + 1) {
 		for (nxt = env; *nxt; ++nxt) {
 			if (nxt >= &environment.data[ENV_SIZE]) {
 				fprintf(stderr, "## Error: "
@@ -614,7 +641,14 @@ int fw_env_write(char *name, char *value)
 	/*
 	 * Append new definition at the end
 	 */
-	for (env = environment.data; *env || *(env + 1); ++env)
+	for (nxt = environment.data; !(*nxt); ++nxt) {
+		if (nxt >= &environment.data[ENV_SIZE]) {
+			fprintf (stderr, "## Error: "
+				"environment is empty\n");
+			return -1;
+		}
+	}
+	for (env = nxt; *env || *(env + 1); ++env)
 		;
 	if (env > environment.data)
 		++env;
-- 
1.9.1

[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[Stefan Agner]

Maybe I miss something, but why is this needed exactly?


--

Stefan

On 27.11.18 02:49, zqb-all wrote:
> Signed-off-by: zqb-all <zhuangqiubin@gmail.com>
> ---
> v2: fix all "env = environment.data" and "env = default_environment"
>
>  tools/env/fw_env.c | 46 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index a5d7595..60742b6 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -398,7 +398,14 @@ char *fw_getenv(char *name)
>  {
>  	char *env, *nxt;
>  
> -	for (env = environment.data; *env; env = nxt + 1) {
> +	for (nxt = environment.data; !(*nxt); ++nxt) {
> +		if (nxt >= &environment.data[ENV_SIZE]) {
> +			fprintf (stderr, "## Error: "
> +				"environment is empty\n");
> +			return NULL;
> +		}
> +	}
> +	for (env = nxt; *env; env = nxt + 1) {
>  		char *val;
>  
>  		for (nxt = env; *nxt; ++nxt) {
> @@ -423,8 +430,14 @@ char *fw_getenv(char *name)
>  char *fw_getdefenv(char *name)
>  {
>  	char *env, *nxt;
> -
> -	for (env = default_environment; *env; env = nxt + 1) {
> +	for (nxt = default_environment; !(*nxt); ++nxt) {
> +		if (nxt >= &default_environment[ENV_SIZE]) {
> +			fprintf (stderr, "## Error: "
> +				"default environment is empty\n");
> +			return NULL;
> +		}
> +	}
> +	for (env = nxt; *env; env = nxt + 1) {
>  		char *val;
>  
>  		for (nxt = env; *nxt; ++nxt) {
> @@ -464,7 +477,14 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
>  
>  	if (argc == 0) {	/* Print all env variables  */
>  		char *env, *nxt;
> -		for (env = environment.data; *env; env = nxt + 1) {
> +		for (nxt = environment.data; !(*nxt); ++nxt) {
> +			if (nxt >= &environment.data[ENV_SIZE]) {
> +				fprintf (stderr, "## Error: "
> +					"environment is empty\n");
> +				return -1;
> +			}
> +		}
> +		for (env = nxt; *env; env = nxt + 1) {
>  			for (nxt = env; *nxt; ++nxt) {
>  				if (nxt >= &environment.data[ENV_SIZE]) {
>  					fprintf(stderr, "## Error: "
> @@ -537,7 +557,14 @@ int fw_env_write(char *name, char *value)
>  	/*
>  	 * search if variable with this name already exists
>  	 */
> -	for (nxt = env = environment.data; *env; env = nxt + 1) {
> +	for (nxt = environment.data; !(*nxt); ++nxt) {
> +		if (nxt >= &environment.data[ENV_SIZE]) {
> +			fprintf (stderr, "## Error: "
> +				"environment is empty\n");
> +			return -1;
> +		}
> +	}
> +	for (env = nxt; *env; env = nxt + 1) {
>  		for (nxt = env; *nxt; ++nxt) {
>  			if (nxt >= &environment.data[ENV_SIZE]) {
>  				fprintf(stderr, "## Error: "
> @@ -614,7 +641,14 @@ int fw_env_write(char *name, char *value)
>  	/*
>  	 * Append new definition at the end
>  	 */
> -	for (env = environment.data; *env || *(env + 1); ++env)
> +	for (nxt = environment.data; !(*nxt); ++nxt) {
> +		if (nxt >= &environment.data[ENV_SIZE]) {
> +			fprintf (stderr, "## Error: "
> +				"environment is empty\n");
> +			return -1;
> +		}
> +	}
> +	for (env = nxt; *env || *(env + 1); ++env)
>  		;
>  	if (env > environment.data)
>  		++env;

[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[Wolfgang Denk]

Dear zqb-all,

In message <1543283392-31275-1-git-send-email-zhuangqiubin@gmail.com> you wrote:
> Signed-off-by: zqb-all <zhuangqiubin@gmail.com>

Umm... please add a commit message that explains which problem you
are trying to fix, and why you think your new code helps to address
this problem.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
One possible reason that things aren't going  according  to  plan  is
that there never was a plan in the first place.

[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[zqb-all]

Hi Wolfgang , Stefan

I try to use uboot envtools in allwinner's platform, then found that the
env in allwinner's SDK have some leading zero, that prevent envtools to
parse it.
Finally I add these code to fix it.

Wolfgang Denk <wd@denx.de> 于2018年11月27日周二 下午11:00写道：

> Dear zqb-all,
>
> In message <1543283392-31275-1-git-send-email-zhuangqiubin@gmail.com> you
> wrote:
> > Signed-off-by: zqb-all <zhuangqiubin@gmail.com>
>
> Umm... please add a commit message that explains which problem you
> are trying to fix, and why you think your new code helps to address
> this problem.
>
> Thanks.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> One possible reason that things aren't going  according  to  plan  is
> that there never was a plan in the first place.
>


-- 

Best regards

[U-Boot] [PATCH v2] tools: env: support skip leading zero of env

[Wolfgang Denk]

Dear 庄秋彬,

In message <CAHaqGhZq+qLNNUbS1UnPjQDYepf=aUqkdLUeZ0g+B0kYNNg96A@mail.gmail.com> you wrote:
>
> I try to use uboot envtools in allwinner's platform, then found that the
> env in allwinner's SDK have some leading zero, that prevent envtools to
> parse it. Finally I add these code to fix it.

This is the wrong approach.

Either you misunderstand the format (for example in case of a
redundant environment where you have to take the flag byte into
consideration), or there is a bug in the implementation of the
Allwinner SDK if it creates such a an environment.

In any case, it is not legal for the U-Boot environment to have any
leading zeros.  By definition, a sequence of two NUL-Bytes will mark
the end of the environment.

You should fix the cause of the problem, and not modify the code to
work on random / incorrect data.

Naked-by: Wolfgang Denk <wd@denx.de>

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anyone can count the seeds in an apple.
No one can count the apples in a seed.