[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[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.

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",
-- 
2.1.4

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Philippe De Swert]

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

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Marek Vasut]

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

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Wolfgang Denk]

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

-- 
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

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Tom Rini]

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.

-- 
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>

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Marek Vasut]

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

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Tom Rini]

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!

-- 
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>

[U-Boot] [PATCH] mkimage: Fix generating multi and script images again

[Philippe De Swert]

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