mkimage: Fix generating multi and script images again

PATCH New Marek Vasut 4 participants 8 messages Dec 7, 2015

Share & tools

Marek Vasut

author

2015-12-07 17:01 UTC #0 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

Seems 6ae6e160 broke creating multi and script type images and even building of mkimage itself. There are two problems with that patch.

First is that expression (!(x == 0) || !(x == 1)) is always true for unsigned int x. The expression must use AND (&&) not OR (||) to be correct.

Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct. The parenthesis are added for clarity.

Signed-off-by Marek Vasut <[email protected]>

Cc Tom Rini <[email protected]>

Cc Philippe De Swert <[email protected]>

Cc Simon Glass <[email protected]>

---
 tools/mkimage.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

tools/mkimage.c +1 -2

index ae01cb1..8f8b6df 100644
@@ -311,8 +311,7 @@ NXTARG:		;
 		exit (retval);
 	}
 
-	if (!params.type == IH_TYPE_MULTI ||
-	    !params.type == IH_TYPE_SCRIPT) {
+	if ((params.type != IH_TYPE_MULTI) && (params.type != IH_TYPE_SCRIPT)) {
 		dfd = open(params.datafile, O_RDONLY | O_BINARY);
 		if (dfd < 0) {
 			fprintf(stderr, "%s: Can't open %s: %s\n",

Signature · 1 line

2.1.4

Philippe De Swert

2015-12-07 17:18 UTC #1 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

Hi,

I haven't had time to check the previous report yet.

Quoted 6 lines · Marek Vasut

Seems 6ae6e160 broke creating multi and script type images and even building of mkimage itself. There are two problems with that patch.
First is that expression (!(x == 0) || !(x == 1)) is always true for unsigned int x. The expression must use AND (&&) not OR (||) to be correct.

It is either multi or script, so AND does not sound correct. The code should skip the following bit if either of those
flags is detected. I admit I threw in the script bit as an afterthought and things went wrong there.

Correct would be if( !(params.type == IH_TYPE_MULTI || params.type == IH_TYPE_SCRIPT))

I'll double-check stuff and submit a new patch

Quoted 4 lines

Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct. The parenthesis are added for clarity.

Weirdly enough I have gcc 4.9.2 and it did not even beep, so I don't know how it could have broken the build.
Give me some time to submit a corrective patch later tonight.

Cheers,

Philippe

Marek Vasut

author

2015-12-07 17:32 UTC #2 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

Quoted 17 lines · Philippe De Swert

Hi,
I haven't had time to check the previous report yet.
Quoted 6 lines · Marek Vasut
Seems 6ae6e160 broke creating multi and script type images and even building of mkimage itself. There are two problems with that patch.
First is that expression (!(x == 0) || !(x == 1)) is always true for unsigned int x. The expression must use AND (&&) not OR (||) to be correct.
It is either multi or script, so AND does not sound correct. The code should skip the following bit if either of those
flags is detected. I admit I threw in the script bit as an afterthought and things went wrong there.

Quoted 4 lines

Correct would be if( !(params.type == IH_TYPE_MULTI || params.type == IH_TYPE_SCRIPT))
I'll double-check stuff and submit a new patch

So yeah, !(X or Y) <=> (!X and !Y) . The patch does that. See
https://en.wikipedia.org/wiki/De_Morgan%27s_laws

Quoted 8 lines

Quoted 4 lines
Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct. The parenthesis are added for clarity.
Weirdly enough I have gcc 4.9.2 and it did not even beep, so I don't know how it could have broken the build.
Give me some time to submit a corrective patch later tonight.

This patch should fix things.

Best regards,
Marek Vasut

Wolfgang Denk

2015-12-07 17:47 UTC #3 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

In reply to Marek Vasut

Dear Marek,

Quoted 4 lines · In message <[email protected]> you wrote:

Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct.

But (!x == 0) && (!x == 1) ist not the same as (x != 0) && (x != 1); assume x=2:

	(!2 == 0) && (!2 == 1) => (0 == 0) && (0 == 1) => 1 && 0 => 0

	(2 != 0) && (2 != 1) => 1 && 1 => 1

???

Best regards,

Wolfgang Denk

Signature · 5 lines

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
A weak mind is like a microscope, which magnifies trifling things,
but cannot receive great ones.      -- Philip Earl of Chesterfield

Tom Rini

2015-12-07 18:03 UTC #4 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

Quoted 7 lines · Wolfgang Denk

Dear Marek,
Quoted 4 lines · In message <[email protected]> you wrote:
Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct.

ok, part of the problem is that we aren't testing !x == 0 && !x == 1 (and I'm re-wording the commit msg, we had been talking about this on IRC) but "!x == 4 || !x == 6".

Quoted 2 lines

But (!x == 0) && (!x == 1) ist not the same as (x != 0) && (x != 1); assume x=2:

... so this is a different thing to consider too.

I'm re-wording things because in sum, what Philippe did is not straight forward, and Marek's version is.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151207/96160252/attachment.sig>

Marek Vasut

author

2015-12-07 18:05 UTC #5 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

In reply to Wolfgang Denk

Quoted 7 lines · Wolfgang Denk

Dear Marek,
Quoted 3 lines · In message <[email protected]> you wrote:
Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct.

Quoted 8 lines

But (!x == 0) && (!x == 1) ist not the same as (x != 0) && (x != 1); assume x=2:
	(!2 == 0) && (!2 == 1) => (0 == 0) && (0 == 1) => 1 && 0 => 0

	(2 != 0) && (2 != 1) => 1 && 1 => 1
???

This really depends on where you put the parenthesis and GCC complains about such ambiguous expressions. That's what the paragraph is about. I never said that ((!x) == 0) && ((!x) == 1) <=> (x != 0) && (x != 1) or equally ((!x) == 0) && ((!x) == 1) <=> !(x == 0) && !(x == 1)

Best regards,
Marek Vasut

Tom Rini

2015-12-07 19:06 UTC #6 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

In reply to Marek Vasut

Quoted 12 lines · Marek Vasut

Seems 6ae6e160 broke creating multi and script type images and even building of mkimage itself. There are two problems with that patch.
First is that expression (!(x == 0) || !(x == 1)) is always true for unsigned int x. The expression must use AND (&&) not OR (||) to be correct.
Second is the coding which causes gcc 4.9.x and newer scream gruesome death and murder. The expression !x == 0 && !x == 1 is ambiguous and should instead be rewritten into (x != 0) && (x != 1) to be correct. The parenthesis are added for clarity.

Quoted 4 lines

Signed-off-by Marek Vasut <[email protected]>
Cc Tom Rini <[email protected]>
Cc Philippe De Swert <[email protected]>
Cc Simon Glass <[email protected]>

After rewording the commit message a bit (and talking with Marek on IRC about my reword), applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151207/9022f4ca/attachment.sig>

Philippe De Swert

2015-12-07 20:52 UTC #7 lore raw

PATCH 1/1 mkimage: Fix generating multi and script images again

Hi,

Quoted 5 lines · Tom Rini

Quoted 3 lines · Marek Vasut
Seems 6ae6e160 broke creating multi and script type images and even building of mkimage itself. There are two problems with that patch.

Quoted 7 lines

Quoted 7 lines
First is that expression (!(x == 0) || !(x == 1)) is always true for unsigned int x. The expression must use AND (&&) not OR (||) to be correct.
After rewording the commit message a bit (and talking with Marek on IRC about my reword), applied to u-boot/master, thanks!

Great! Apologies for the breakage,

Philippe