[U-Boot] [PATCH] tools/env: fix redundant env flag comparison

[U-Boot] [PATCH] tools/env: fix redundant env flag comparison

[Jon Povey]

This fixes two bugs with comparison of redundant environment flags on
read.

flag0 and flag1 in fw_env_open() were declared signed instead of
unsigned char breaking BOOLEAN mode "== 0xFF" tests and in INCREMENTAL
mode the wrong environment would be chosen where the flag values are
127 and 128 (either way round). With both flags over 128, both signs
flipped and the logic worked by happy accident.

Also there was a logic bug in the INCREMENTAL test (after signedness was
fixed) in the case flag0=0, flag1=255, env 1 would be incorrectly chosen.

Fix both of these.

Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
---
After my gripe yesterday, something more constructive:

I confirmed these bugs with a little test program in C, can send that
if anyone wants it.

At a glance it looks like the main u-boot env handling does not have
these bugs.

 tools/env/fw_env.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 8ff7052..315d7ce 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1067,11 +1067,11 @@ static char *envmatch (char * s1, char * s2)
 int fw_env_open(void)
 {
 	int crc0, crc0_ok;
-	char flag0;
+	unsigned char flag0;
 	void *addr0;
 
 	int crc1, crc1_ok;
-	char flag1;
+	unsigned char flag1;
 	void *addr1;
 
 	struct env_image_single *single;
@@ -1182,14 +1182,13 @@ int fw_env_open(void)
 				}
 				break;
 			case FLAG_INCREMENTAL:
-				if ((flag0 == 255 && flag1 == 0) ||
-				    flag1 > flag0)
+				if (flag0 == 255 && flag1 == 0)
 					dev_current = 1;
 				else if ((flag1 == 255 && flag0 == 0) ||
-					 flag0 > flag1)
-					dev_current = 0;
-				else /* flags are equal - almost impossible */
+					 flag0 >= flag1)
 					dev_current = 0;
+				else /* flag1 > flag0 */
+					dev_current = 1;
 				break;
 			default:
 				fprintf (stderr, "Unknown flag scheme %u \n",
-- 
1.6.3.3

[U-Boot] [PATCH] tools/env: fix redundant env flag comparison

[Wolfgang Denk]

Dear Jon Povey,

In message <1299820256-29757-1-git-send-email-jon.povey@racelogic.co.uk> you wrote:
> This fixes two bugs with comparison of redundant environment flags on
> read.
> 
> flag0 and flag1 in fw_env_open() were declared signed instead of
> unsigned char breaking BOOLEAN mode "== 0xFF" tests and in INCREMENTAL
> mode the wrong environment would be chosen where the flag values are
> 127 and 128 (either way round). With both flags over 128, both signs
> flipped and the logic worked by happy accident.
> 
> Also there was a logic bug in the INCREMENTAL test (after signedness was
> fixed) in the case flag0=0, flag1=255, env 1 would be incorrectly chosen.
> 
> Fix both of these.
> 
> Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
> ---
> After my gripe yesterday, something more constructive:
> 
> I confirmed these bugs with a little test program in C, can send that
> if anyone wants it.
> 
> At a glance it looks like the main u-boot env handling does not have
> these bugs.
> 
>  tools/env/fw_env.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
Repeat after me:
Usenet is not a word processor; it's a medium where aesthetics count.
Mozilla is not a newsreader; it's a web browser.
Windows is not an operating system; it's a GUI on a program loader.
         -- Tom Christiansen in <6bq0g5$lr4$2@csnews.cs.colorado.edu>