[U-Boot] [PATCH] 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 marex@denx.de Cc: Tom Rini trini@konsulko.com Cc: Philippe De Swert philippedeswert@gmail.com Cc: Simon Glass sjg@chromium.org --- tools/mkimage.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c index ae01cb1..8f8b6df 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -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",
Hi,
I haven't had time to check the previous report yet.
On 07/12/15 19:01, Marek Vasut wrote:
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
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
On Monday, December 07, 2015 at 06:18:02 PM, Philippe De Swert wrote:
Hi,
I haven't had time to check the previous report yet.
On 07/12/15 19:01, Marek Vasut wrote:
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
So yeah, !(X or Y) <=> (!X and !Y) . The patch does that. See https://en.wikipedia.org/wiki/De_Morgan%27s_laws
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
Dear Marek,
In message 1449507714-9599-1-git-send-email-marex@denx.de 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
On Mon, Dec 07, 2015 at 06:47:36PM +0100, Wolfgang Denk wrote:
Dear Marek,
In message 1449507714-9599-1-git-send-email-marex@denx.de 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".
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.
On Monday, December 07, 2015 at 06:47:36 PM, Wolfgang Denk wrote:
Dear Marek,
In message 1449507714-9599-1-git-send-email-marex@denx.de 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
???
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
On Mon, Dec 07, 2015 at 06:01:54PM +0100, Marek Vasut wrote:
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 marex@denx.de Cc: Tom Rini trini@konsulko.com Cc: Philippe De Swert philippedeswert@gmail.com Cc: Simon Glass sjg@chromium.org
After rewording the commit message a bit (and talking with Marek on IRC about my reword), applied to u-boot/master, thanks!
Hi,
On 07/12/15 21:06, Tom Rini wrote:
On Mon, Dec 07, 2015 at 06:01:54PM +0100, Marek Vasut wrote:
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.
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
participants (4)
-
Marek Vasut -
Philippe De Swert -
Tom Rini -
Wolfgang Denk