Not successful if statements returning error code

DISCUSSION Andrej Picej 3 participants 7 messages Jul 23, 2021 – Aug 9, 2021

Share & tools

Andrej Picej

author

2021-07-23 10:20 UTC #0 lore raw

Hi all,

I have a question about Hush shell and the use of its conditional statements. We have come upon an interesting behaviour with its return values.

If the condition of the if statement is not true then the return value is 1 (error code), and if the condition is true the return value is 0 (success).

Simple test:

Quoted 10 lines

#!/bin/sh
if [ 0 -gt 1 ]; then
echo "0 gt 1, Ret: $?"
fi
echo "Ret: $?"
if [ 2 -gt 1 ]; then
echo "2 gt 1, Ret: $?"
fi
echo "Ret: $?"

Output:

Quoted 3 lines

Ret: 1
2 gt 1, Ret: 0
Ret: 0

This means that if, for example the first if statement (where condition is not met) would be at the end of a script the return value of that whole script would be 1 (error code). I don't think this follows standard shell behaviour. If if statement condition is not met this doesn't mean that the return value should be an error code, right? Using other shells (bash for example) we can see that the returned value in both cases is 0, which is expected (IMO).

This behaviour is not new to the Hush or barebox as I could reproduce it on various previous barebox versions (2013.08.0, 2017.12.0 and 2019.11.0).

Of course, this problem can be easily avoided if at the end of every script we use explicit exit 0. This is doable, but a little annoying.

Although this is not a deal-breaker for us, I was wondering what is the reason behind this? How do you get around this and are there any plans to fix/modify this in the future so it follows the behaviour of other shells?

Thank you for your answer.

Best regards,
Andrej

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Andrej Picej

author

2021-07-27 07:17 UTC #1 lore raw

Just saw that I forgot to mention the main reason, why this is important.
We have a couple of boot scripts which end with if statements. If conditions of these if statements are not met then the boot script returns error code and the booting process is broken:

Quoted 3 lines

Running script '/env/boot/mmc' failed: error -1
Booting entry 'mmc' failed
Nothing bootable found

BR,
Andrej

Quoted 36 lines · Andrej Picej

Hi all,
I have a question about Hush shell and the use of its conditional statements. We have come upon an interesting behaviour with its return values.
If the condition of the if statement is not true then the return value is 1 (error code), and if the condition is true the return value is 0 (success).
Simple test:
Quoted 10 lines
#!/bin/sh
if [ 0 -gt 1 ]; then
echo "0 gt 1, Ret: $?"
fi
echo "Ret: $?"
if [ 2 -gt 1 ]; then
echo "2 gt 1, Ret: $?"
fi
echo "Ret: $?"
Output:
Quoted 3 lines
Ret: 1
2 gt 1, Ret: 0
Ret: 0
This means that if, for example the first if statement (where condition is not met) would be at the end of a script the return value of that whole script would be 1 (error code). I don't think this follows standard shell behaviour. If if statement condition is not met this doesn't mean that the return value should be an error code, right? Using other shells (bash for example) we can see that the returned value in both cases is 0, which is expected (IMO).
This behaviour is not new to the Hush or barebox as I could reproduce it on various previous barebox versions (2013.08.0, 2017.12.0 and 2019.11.0).
Of course, this problem can be easily avoided if at the end of every script we use explicit exit 0. This is doable, but a little annoying.
Although this is not a deal-breaker for us, I was wondering what is the reason behind this? How do you get around this and are there any plans to fix/modify this in the future so it follows the behaviour of other shells?
Thank you for your answer.

Quoted 7 lines

Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Ahmad Fatoum

2021-07-28 08:58 UTC #2 lore raw

In reply to Andrej Picej

Hello Andrej,

Quoted 28 lines · Andrej Picej

Hi all,
I have a question about Hush shell and the use of its conditional statements. We have come upon an interesting behaviour with its return values.
If the condition of the if statement is not true then the return value is 1 (error code), and if the condition is true the return value is 0 (success).
Simple test:
Quoted 10 lines
#!/bin/sh
if [ 0 -gt 1 ]; then
echo "0 gt 1, Ret: $?"
fi
echo "Ret: $?"
if [ 2 -gt 1 ]; then
echo "2 gt 1, Ret: $?"
fi
echo "Ret: $?"
Output:
Quoted 3 lines
Ret: 1
2 gt 1, Ret: 0
Ret: 0
This means that if, for example the first if statement (where condition is not met) would be at the end of a script the return value of that whole script would be 1 (error code). I don't think this follows standard shell behaviour. If if statement condition is not met this doesn't mean that the return value should be an error code, right? Using other shells (bash for example) we can see that the returned value in both cases is 0, which is expected (IMO).

Quoted 5 lines

This behaviour is not new to the Hush or barebox as I could reproduce it on various previous barebox versions (2013.08.0, 2017.12.0 and 2019.11.0).
Of course, this problem can be easily avoided if at the end of every script we use explicit exit 0. This is doable, but a little annoying.
Although this is not a deal-breaker for us, I was wondering what is the reason behind this? How do you get around this and are there any plans to fix/modify this in the future so it follows the behaviour of other shells?

I talked with Sascha once before (off-list) about a similar issue. Missing error handling results in

  [ x1 -eq x2 ] && echo yes || echo no

printing yes.

We agreed that it's ok to break scripts relying on clear hush bugs like this.

I'd thus recommend you to send a patch to fix hush instead of working around it.

Cheers,
Ahmad

Quoted 11 lines

Thank you for your answer.
Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Signature · 9 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Denis Osterland-Heim

2021-07-28 09:40 UTC #3 lore raw

Hi,

Am Mittwoch, den 28.07.2021, 10:56 +0200 schrieb Ahmad Fatoum:

Quoted 41 lines

Hello Andrej,
Quoted 28 lines · Andrej Picej
Hi all,
I have a question about Hush shell and the use of its conditional statements. We have come upon an interesting behaviour with its return values.
If the condition of the if statement is not true then the return value is 1 (error code), and if the condition is true the return value is 0 (success).
Simple test:
Quoted 10 lines
#!/bin/sh
if [ 0 -gt 1 ]; then
echo "0 gt 1, Ret: $?"
fi
echo "Ret: $?"
if [ 2 -gt 1 ]; then
echo "2 gt 1, Ret: $?"
fi
echo "Ret: $?"
Output:
Quoted 3 lines
Ret: 1
2 gt 1, Ret: 0
Ret: 0
This means that if, for example the first if statement (where condition is not met) would be at the end of a script the return value of that whole script would be 1 (error code). I don't think this follows standard shell behaviour. If if statement condition is not met this doesn't mean that the return value should be an error code, right? Using other shells (bash for example) we can see that the returned value in both cases is 0, which is expected (IMO).
Quoted 6 lines
This behaviour is not new to the Hush or barebox as I could reproduce it on various previous barebox versions (2013.08.0, 2017.12.0 and 2019.11.0).
Of course, this problem can be easily avoided if at the end of every script we use explicit exit 0. This is doable, but a little annoying.
Although this is not a deal-breaker for us, I was wondering what is the reason behind this? How do you get around this and are there any plans to fix/modify this in the future so it follows the behaviour of other shells?
I talked with Sascha once before (off-list) about a similar issue. Missing error handling results in

Quoted 7 lines

  [ x1 -eq x2 ] && echo yes || echo no
printing yes.
We agreed that it's ok to break scripts relying on clear hush bugs like this.
I'd thus recommend you to send a patch to fix hush instead of working around it.

shellcheck says:

SC2015: Note that A && B || C is not if-then-else. C may run when A is true.

So I would think that B is maybe allowed to run before A, too. But, I do not know if Hush specifies execution order here.

Regards, Denis

Quoted 17 lines

Cheers,
Ahmad
Quoted 11 lines
Thank you for your answer.
Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315

________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.

- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:

https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

- For general information on data protection and your respective rights please visit:

https://www.diehl.com/group/en/transparency-and-information-obligations/

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Ahmad Fatoum

2021-07-29 09:24 UTC #4 lore raw

Quoted 13 lines · Denis Osterland-Heim

Hi,
Am Mittwoch, den 28.07.2021, 10:56 +0200 schrieb Ahmad Fatoum:
Quoted 9 lines
I talked with Sascha once before (off-list) about a similar issue. Missing error handling results in
  [ x1 -eq x2 ] && echo yes || echo no
printing yes.
We agreed that it's ok to break scripts relying on clear hush bugs like this.

Quoted 7 lines

Quoted 1 line
I'd thus recommend you to send a patch to fix hush instead of working around it.
shellcheck says:
SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
So I would think that B is maybe allowed to run before A, too. But, I do not know if Hush specifies execution order here.

An option-less barebox echo can't fail, so it should never print both yes and no, but thanks for the reference. I'll keep an eye on it in future shell code.

This wasn't what I meant to show though, I meant that x1 and x2 are both non-integers and -eq which compares integers returns true when comparing them unlike what a POSIX shell does.

Cheers,
Ahmad

Quoted 48 lines

Regards, Denis
Quoted 17 lines
Cheers,
Ahmad
Quoted 11 lines
Thank you for your answer.
Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315
________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:
https://www.diehl.com/group/de/transparenz-und-informationspflichten/
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
- For general information on data protection and your respective rights please visit:
https://www.diehl.com/group/en/transparency-and-information-obligations/
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Signature · 9 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Denis Osterland-Heim

2021-07-29 09:49 UTC #5 lore raw

Hi,

Am Donnerstag, den 29.07.2021, 11:22 +0200 schrieb Ahmad Fatoum:

Quoted 23 lines · Denis Osterland-Heim

Quoted 13 lines
Hi,
Am Mittwoch, den 28.07.2021, 10:56 +0200 schrieb Ahmad Fatoum:
Quoted 9 lines
I talked with Sascha once before (off-list) about a similar issue. Missing error handling results in
  [ x1 -eq x2 ] && echo yes || echo no
printing yes.
We agreed that it's ok to break scripts relying on clear hush bugs like this.
Quoted 8 lines
Quoted 1 line
I'd thus recommend you to send a patch to fix hush instead of working around it.
shellcheck says:
SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
So I would think that B is maybe allowed to run before A, too. But, I do not know if Hush specifies execution order here.

Quoted 7 lines

An option-less barebox echo can't fail, so it should never print both yes and no, but thanks for the reference. I'll keep an eye on it in future shell code.
This wasn't what I meant to show though, I meant that x1 and x2 are both non-integers and -eq which compares integers returns true when comparing them unlike what a POSIX shell does.

Oh. Now I got your point ;-)

https://git.pengutronix.de/cgit/barebox/tree/commands/test.c#n182
error handling is missing here.
It is very likely that simple_strtol() returns the same value in case of error, which results in a successfully compare.

Regards, Denis

Quoted 56 lines

Cheers,
Ahmad
Quoted 49 lines
Regards, Denis
Quoted 17 lines
Cheers,
Ahmad
Quoted 11 lines
Thank you for your answer.
Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315
________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:
https://www.diehl.com/group/de/transparenz-und-informationspflichten/
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
- For general information on data protection and your respective rights please visit:
https://www.diehl.com/group/en/transparency-and-information-obligations/
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315

________________________________

- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:

https://www.diehl.com/group/de/transparenz-und-informationspflichten/

- For general information on data protection and your respective rights please visit:

https://www.diehl.com/group/en/transparency-and-information-obligations/

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Ahmad Fatoum

2021-08-09 10:56 UTC #6 lore raw

Hi,

Quoted 33 lines · Denis Osterland-Heim

Hi,
Am Donnerstag, den 29.07.2021, 11:22 +0200 schrieb Ahmad Fatoum:
Quoted 23 lines · Denis Osterland-Heim
Quoted 13 lines
Hi,
Am Mittwoch, den 28.07.2021, 10:56 +0200 schrieb Ahmad Fatoum:
Quoted 9 lines
I talked with Sascha once before (off-list) about a similar issue. Missing error handling results in
  [ x1 -eq x2 ] && echo yes || echo no
printing yes.
We agreed that it's ok to break scripts relying on clear hush bugs like this.
Quoted 8 lines
Quoted 1 line
I'd thus recommend you to send a patch to fix hush instead of working around it.
shellcheck says:
SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
So I would think that B is maybe allowed to run before A, too. But, I do not know if Hush specifies execution order here.
Quoted 7 lines
An option-less barebox echo can't fail, so it should never print both yes and no, but thanks for the reference. I'll keep an eye on it in future shell code.
This wasn't what I meant to show though, I meant that x1 and x2 are both non-integers and -eq which compares integers returns true when comparing them unlike what a POSIX shell does.

Quoted 6 lines

Oh. Now I got your point ;-)
https://git.pengutronix.de/cgit/barebox/tree/commands/test.c#n182
error handling is missing here.
It is very likely that simple_strtol() returns the same value in case of error, which results in a successfully compare.

Yes. I noted this because of the similarity to Andrej's problem:
A bug leads to clearly bogus shell behavior. It can be ok to fix such bugs despite them not being 100% backwards compatible.

Quoted 82 lines

Regards, Denis
Quoted 56 lines
Cheers,
Ahmad
Quoted 49 lines
Regards, Denis
Quoted 17 lines
Cheers,
Ahmad
Quoted 11 lines
Thank you for your answer.
Best regards,
Andrej
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315
________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:
https://www.diehl.com/group/de/transparenz-und-informationspflichten/
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
- For general information on data protection and your respective rights please visit:
https://www.diehl.com/group/en/transparency-and-information-obligations/
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315
________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:
https://www.diehl.com/group/de/transparenz-und-informationspflichten/
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
- For general information on data protection and your respective rights please visit:
https://www.diehl.com/group/en/transparency-and-information-obligations/

Signature · 9 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox