| Message ID | 1521108536-32693-1-git-send-email-luca@lucaceresoli.net |
|---|---|
| State | Accepted |
| Commit | f69dce5081396a83b87d6df2693764b99466a18d |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] scripts/check-config.sh: fix "command not found" error handling | expand |
On 15 March 2018 at 04:08, Luca Ceresoli <luca@lucaceresoli.net> wrote: > scripts/check-config.sh exits successfully and silently without doing > any checks when the 'comm' command is not found. > > The problem triggers from the command around line 39: > > comm -23 ${suspects} ${ok} >${new_adhoc} > > This statement fails when 'comm' is not in $PATH, creating an empty > ${new_adhoc} file. But the script continues and the following line, > which is supposed to detect an error: > > if [ -s ${new_adhoc} ]; then > > will always be false since the file is empty, and the script will exit > successfully as if everything were OK. > > The case where 'comm' in not in $PATH is not theoretical. It used to > happen on yocto until a recent fix [0], and still happens on the > current stable branch (rocko). > > Fix by setting the errexit flag to exit with error when a statement > fails, so that at least the problem is noticed. > > For additional safety also set the nounset flag to detect expansion > errors. > > [0] http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=fe0b4cb5b48580d4a3f3c0eb82bfa6f1b13801e4 > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > --- > scripts/check-config.sh | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> I wonder if we should have an explicit || die "msg" at the end of the command that can fail, but this seems OK too. Regards, Simon
On Thu, Mar 15, 2018 at 11:08:56AM +0100, Luca Ceresoli wrote: > scripts/check-config.sh exits successfully and silently without doing > any checks when the 'comm' command is not found. > > The problem triggers from the command around line 39: > > comm -23 ${suspects} ${ok} >${new_adhoc} > > This statement fails when 'comm' is not in $PATH, creating an empty > ${new_adhoc} file. But the script continues and the following line, > which is supposed to detect an error: > > if [ -s ${new_adhoc} ]; then > > will always be false since the file is empty, and the script will exit > successfully as if everything were OK. > > The case where 'comm' in not in $PATH is not theoretical. It used to > happen on yocto until a recent fix [0], and still happens on the > current stable branch (rocko). > > Fix by setting the errexit flag to exit with error when a statement > fails, so that at least the problem is noticed. > > For additional safety also set the nounset flag to detect expansion > errors. > > [0] http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=fe0b4cb5b48580d4a3f3c0eb82bfa6f1b13801e4 > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 267758498b3d..4848ca6e25a5 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -14,6 +14,9 @@ # For example: # scripts/check-config.sh b/chromebook_link/u-boot.cfg kconfig_whitelist.txt . +set -e +set -u + path="$1" whitelist="$2" srctree="$3"
scripts/check-config.sh exits successfully and silently without doing any checks when the 'comm' command is not found. The problem triggers from the command around line 39: comm -23 ${suspects} ${ok} >${new_adhoc} This statement fails when 'comm' is not in $PATH, creating an empty ${new_adhoc} file. But the script continues and the following line, which is supposed to detect an error: if [ -s ${new_adhoc} ]; then will always be false since the file is empty, and the script will exit successfully as if everything were OK. The case where 'comm' in not in $PATH is not theoretical. It used to happen on yocto until a recent fix [0], and still happens on the current stable branch (rocko). Fix by setting the errexit flag to exit with error when a statement fails, so that at least the problem is noticed. For additional safety also set the nounset flag to detect expansion errors. [0] http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=fe0b4cb5b48580d4a3f3c0eb82bfa6f1b13801e4 Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> --- scripts/check-config.sh | 3 +++ 1 file changed, 3 insertions(+)