* [PATCH] board: toradex: Quote variables in `test` cmd expression
@ 2026-03-31 8:10 Franz Schnyder
2026-04-01 5:10 ` Francesco Dolcini
0 siblings, 1 reply; 5+ messages in thread
From: Franz Schnyder @ 2026-03-31 8:10 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Francesco Dolcini, Rasmus Villemoes, Franz Schnyder
From: Franz Schnyder <franz.schnyder@toradex.com>
With correct POSIX handling, unquoted empty variables can turn the
expression like
test -n ${fdtfile}
into
test -n
The POSIX handling for single argument `test` evaluates it as true,
so the fallback initialization will be skipped unexpectedly.
Quoting variable expansions in `test` expressions will always result in
correct behavior for empty and non-empty values.
This change was triggered by
commit 8b0619579b22 ("cmd: test: fix handling of single-argument form of test")
The aim is to have a less fragile codebase that is not dependent on a
quirk of the shell implementation.
Use quoted variable expansions in `test` expressions throughout.
Signed-off-by: Franz Schnyder <franz.schnyder@toradex.com>
---
board/toradex/aquila-am69/aquila-am69.env | 6 +++---
board/toradex/smarc-imx8mp/smarc-imx8mp.env | 2 +-
board/toradex/smarc-imx95/smarc-imx95.env | 2 +-
board/toradex/verdin-am62p/verdin-am62p.env | 6 +++---
configs/apalis-imx8_defconfig | 2 +-
configs/apalis_imx6_defconfig | 2 +-
configs/aquila-am69_a72_defconfig | 2 +-
configs/colibri-imx6ull-emmc_defconfig | 2 +-
configs/colibri-imx6ull_defconfig | 2 +-
configs/colibri-imx8x_defconfig | 2 +-
configs/colibri_imx6_defconfig | 2 +-
configs/colibri_imx7_defconfig | 2 +-
configs/colibri_imx7_emmc_defconfig | 2 +-
configs/colibri_vf_defconfig | 2 +-
configs/toradex-smarc-imx8mp_defconfig | 2 +-
configs/toradex-smarc-imx95_defconfig | 2 +-
configs/verdin-am62_a53_defconfig | 2 +-
configs/verdin-am62p_a53_defconfig | 2 +-
configs/verdin-imx8mm_defconfig | 2 +-
configs/verdin-imx8mp_defconfig | 2 +-
20 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/board/toradex/aquila-am69/aquila-am69.env b/board/toradex/aquila-am69/aquila-am69.env
index f8b7363dcf5..ac2828e0b58 100644
--- a/board/toradex/aquila-am69/aquila-am69.env
+++ b/board/toradex/aquila-am69/aquila-am69.env
@@ -21,21 +21,21 @@ dfu_alt_info_ram=
update_tiboot3=
askenv confirm Did you load tiboot3.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x0 ${blkcnt};
fi
update_tispl=
askenv confirm Did you load tispl.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x400 ${blkcnt};
fi
update_uboot=
askenv confirm Did you load u-boot.img (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x1400 ${blkcnt};
fi
diff --git a/board/toradex/smarc-imx8mp/smarc-imx8mp.env b/board/toradex/smarc-imx8mp/smarc-imx8mp.env
index 58f152e6b51..1480db8e5fc 100644
--- a/board/toradex/smarc-imx8mp/smarc-imx8mp.env
+++ b/board/toradex/smarc-imx8mp/smarc-imx8mp.env
@@ -13,7 +13,7 @@ scriptaddr=0x50280000
update_uboot=
askenv confirm Did you load flash.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt
${blkcnt} / 0x200; mmc dev 0 1; mmc write ${loadaddr} 0x0
${blkcnt};
diff --git a/board/toradex/smarc-imx95/smarc-imx95.env b/board/toradex/smarc-imx95/smarc-imx95.env
index b94250bbc52..35d26b7cfba 100644
--- a/board/toradex/smarc-imx95/smarc-imx95.env
+++ b/board/toradex/smarc-imx95/smarc-imx95.env
@@ -13,7 +13,7 @@ scriptaddr=0x9c600000
update_uboot=
askenv confirm Did you load flash.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt
${blkcnt} / 0x200; mmc dev 0 1; mmc write ${loadaddr} 0x0
${blkcnt};
diff --git a/board/toradex/verdin-am62p/verdin-am62p.env b/board/toradex/verdin-am62p/verdin-am62p.env
index f8b7363dcf5..ac2828e0b58 100644
--- a/board/toradex/verdin-am62p/verdin-am62p.env
+++ b/board/toradex/verdin-am62p/verdin-am62p.env
@@ -21,21 +21,21 @@ dfu_alt_info_ram=
update_tiboot3=
askenv confirm Did you load tiboot3.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x0 ${blkcnt};
fi
update_tispl=
askenv confirm Did you load tispl.bin (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x400 ${blkcnt};
fi
update_uboot=
askenv confirm Did you load u-boot.img (y/N)?;
- if test $confirm = y; then
+ if test "$confirm" = y; then
setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200;
mmc dev 0 1; mmc write ${loadaddr} 0x1400 ${blkcnt};
fi
diff --git a/configs/apalis-imx8_defconfig b/configs/apalis-imx8_defconfig
index 6ab4dd3dc57..17090ac682b 100644
--- a/configs/apalis-imx8_defconfig
+++ b/configs/apalis-imx8_defconfig
@@ -25,7 +25,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile ${soc}-apalis${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile ${soc}-apalis${variant}-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2068
CONFIG_LOG=y
diff --git a/configs/apalis_imx6_defconfig b/configs/apalis_imx6_defconfig
index cd9811aca61..791be4df283 100644
--- a/configs/apalis_imx6_defconfig
+++ b/configs/apalis_imx6_defconfig
@@ -31,7 +31,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_BOOTCOMMAND="run distro_bootcmd; usb start; setenv stdout serial,vidconsole; setenv stdin serial,usbkbd"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx6q-apalis${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx6q-apalis${variant}-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=1024
CONFIG_SYS_PBSIZE=1055
CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
diff --git a/configs/aquila-am69_a72_defconfig b/configs/aquila-am69_a72_defconfig
index 0f85b228b06..0237a703813 100644
--- a/configs/aquila-am69_a72_defconfig
+++ b/configs/aquila-am69_a72_defconfig
@@ -34,7 +34,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_BOOTCOMMAND="bootflow scan -b"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile k3-am69-aquila-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile k3-am69-aquila-${fdt_board}.dtb"
CONFIG_LOG=y
# CONFIG_DISPLAY_BOARDINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/colibri-imx6ull-emmc_defconfig b/configs/colibri-imx6ull-emmc_defconfig
index 439ddeb8f0b..c54ff48fdf7 100644
--- a/configs/colibri-imx6ull-emmc_defconfig
+++ b/configs/colibri-imx6ull-emmc_defconfig
@@ -18,7 +18,7 @@ CONFIG_FIT_VERBOSE=y
CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx6ull-colibri${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx6ull-colibri${variant}-${fdt_board}.dtb"
CONFIG_SYS_PBSIZE=547
CONFIG_SYS_CONSOLE_IS_IN_ENV=y
# CONFIG_DISPLAY_BOARDINFO is not set
diff --git a/configs/colibri-imx6ull_defconfig b/configs/colibri-imx6ull_defconfig
index 1c400f9534d..cda7a6ada3a 100644
--- a/configs/colibri-imx6ull_defconfig
+++ b/configs/colibri-imx6ull_defconfig
@@ -21,7 +21,7 @@ CONFIG_BOOTDELAY=1
CONFIG_OF_ENV_SETUP=y
CONFIG_BOOTCOMMAND="run ubiboot || run distro_bootcmd;"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx6ull-colibri${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx6ull-colibri${variant}-${fdt_board}.dtb"
CONFIG_SYS_PBSIZE=547
CONFIG_SYS_CONSOLE_IS_IN_ENV=y
# CONFIG_DISPLAY_BOARDINFO is not set
diff --git a/configs/colibri-imx8x_defconfig b/configs/colibri-imx8x_defconfig
index fd6a6b47fcf..3300922b172 100644
--- a/configs/colibri-imx8x_defconfig
+++ b/configs/colibri-imx8x_defconfig
@@ -26,7 +26,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile ${soc}-colibri-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile ${soc}-colibri-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2068
CONFIG_LOG=y
diff --git a/configs/colibri_imx6_defconfig b/configs/colibri_imx6_defconfig
index 0f5abb51f76..34037e1c134 100644
--- a/configs/colibri_imx6_defconfig
+++ b/configs/colibri_imx6_defconfig
@@ -30,7 +30,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_BOOTCOMMAND="run distro_bootcmd; usb start; setenv stdout serial,vidconsole; setenv stdin serial,usbkbd"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx6dl-colibri${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx6dl-colibri${variant}-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=1024
CONFIG_SYS_PBSIZE=1056
CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
diff --git a/configs/colibri_imx7_defconfig b/configs/colibri_imx7_defconfig
index 2a3cb84c3b6..8a84f49f42f 100644
--- a/configs/colibri_imx7_defconfig
+++ b/configs/colibri_imx7_defconfig
@@ -22,7 +22,7 @@ CONFIG_BOOTDELAY=1
CONFIG_OF_ENV_SETUP=y
CONFIG_BOOTCOMMAND="run ubiboot ; echo ; echo ubiboot failed ; run distro_bootcmd;"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile ${soc}-colibri${variant}-${fdt_board}.dtb "
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile ${soc}-colibri${variant}-${fdt_board}.dtb "
CONFIG_SYS_PBSIZE=544
CONFIG_SYS_CONSOLE_IS_IN_ENV=y
# CONFIG_DISPLAY_BOARDINFO is not set
diff --git a/configs/colibri_imx7_emmc_defconfig b/configs/colibri_imx7_emmc_defconfig
index a95a3214d0c..4c661d4cda1 100644
--- a/configs/colibri_imx7_emmc_defconfig
+++ b/configs/colibri_imx7_emmc_defconfig
@@ -19,7 +19,7 @@ CONFIG_FIT_VERBOSE=y
CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile ${soc}-colibri${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile ${soc}-colibri${variant}-${fdt_board}.dtb"
CONFIG_SYS_PBSIZE=544
CONFIG_SYS_CONSOLE_IS_IN_ENV=y
# CONFIG_DISPLAY_BOARDINFO is not set
diff --git a/configs/colibri_vf_defconfig b/configs/colibri_vf_defconfig
index 1364fe45291..4ce14baef4f 100644
--- a/configs/colibri_vf_defconfig
+++ b/configs/colibri_vf_defconfig
@@ -23,7 +23,7 @@ CONFIG_FDT_FIXUP_PARTITIONS=y
CONFIG_USE_BOOTCOMMAND=y
CONFIG_BOOTCOMMAND="run ubiboot || run distro_bootcmd;"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile ${soc}-colibri-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile ${soc}-colibri-${fdt_board}.dtb"
CONFIG_SYS_PBSIZE=1056
CONFIG_LOGLEVEL=3
# CONFIG_DISPLAY_BOARDINFO is not set
diff --git a/configs/toradex-smarc-imx8mp_defconfig b/configs/toradex-smarc-imx8mp_defconfig
index 13a0e5f028b..7be67df3130 100644
--- a/configs/toradex-smarc-imx8mp_defconfig
+++ b/configs/toradex-smarc-imx8mp_defconfig
@@ -40,7 +40,7 @@ CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_BOOTCOMMAND="bootflow scan -b"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx8mp-toradex-smarc-dev.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx8mp-toradex-smarc-dev.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2081
CONFIG_LOG=y
diff --git a/configs/toradex-smarc-imx95_defconfig b/configs/toradex-smarc-imx95_defconfig
index ad908949b59..4faff7977df 100644
--- a/configs/toradex-smarc-imx95_defconfig
+++ b/configs/toradex-smarc-imx95_defconfig
@@ -37,7 +37,7 @@ CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_BOOTCOMMAND="bootflow scan -b"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx95-toradex-smarc-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx95-toradex-smarc-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2074
CONFIG_LOG=y
diff --git a/configs/verdin-am62_a53_defconfig b/configs/verdin-am62_a53_defconfig
index f04858099da..3f3cd2a53b1 100644
--- a/configs/verdin-am62_a53_defconfig
+++ b/configs/verdin-am62_a53_defconfig
@@ -34,7 +34,7 @@ CONFIG_FIT_VERBOSE=y
CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile k3-am625-verdin-${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile k3-am625-verdin-${variant}-${fdt_board}.dtb"
CONFIG_LOG=y
# CONFIG_DISPLAY_BOARDINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/verdin-am62p_a53_defconfig b/configs/verdin-am62p_a53_defconfig
index 3dbffa78662..6cca7427485 100644
--- a/configs/verdin-am62p_a53_defconfig
+++ b/configs/verdin-am62p_a53_defconfig
@@ -36,7 +36,7 @@ CONFIG_BOOTDELAY=1
CONFIG_OF_BOARD_SETUP_EXTENDED=y
CONFIG_BOOTCOMMAND="bootflow scan -b"
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile k3-am62p5-verdin-${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile k3-am62p5-verdin-${variant}-${fdt_board}.dtb"
CONFIG_LOG=y
# CONFIG_DISPLAY_BOARDINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/verdin-imx8mm_defconfig b/configs/verdin-imx8mm_defconfig
index 45437205837..88f337bd347 100644
--- a/configs/verdin-imx8mm_defconfig
+++ b/configs/verdin-imx8mm_defconfig
@@ -33,7 +33,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx8mm-verdin-${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx8mm-verdin-${variant}-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2081
CONFIG_LOG=y
diff --git a/configs/verdin-imx8mp_defconfig b/configs/verdin-imx8mp_defconfig
index 99749c50194..ca815b9de78 100644
--- a/configs/verdin-imx8mp_defconfig
+++ b/configs/verdin-imx8mp_defconfig
@@ -42,7 +42,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_BOOTDELAY=1
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile imx8mp-verdin-${variant}-${fdt_board}.dtb"
+CONFIG_PREBOOT="test -n \"${fdtfile}\" || setenv fdtfile imx8mp-verdin-${variant}-${fdt_board}.dtb"
CONFIG_SYS_CBSIZE=2048
CONFIG_SYS_PBSIZE=2081
CONFIG_LOG=y
---
base-commit: d9eee3d17882ec40f8ca5e231046bbf287ab4369
change-id: 20260330-fix-test-cmd-empty-a26b5eff3611
Best regards,
--
Franz Schnyder <franz.schnyder@toradex.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] board: toradex: Quote variables in `test` cmd expression
2026-03-31 8:10 [PATCH] board: toradex: Quote variables in `test` cmd expression Franz Schnyder
@ 2026-04-01 5:10 ` Francesco Dolcini
2026-04-02 23:30 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Francesco Dolcini @ 2026-04-01 5:10 UTC (permalink / raw)
To: Franz Schnyder
Cc: u-boot, Tom Rini, Francesco Dolcini, Rasmus Villemoes,
Franz Schnyder
On Tue, Mar 31, 2026 at 10:10:10AM +0200, Franz Schnyder wrote:
> From: Franz Schnyder <franz.schnyder@toradex.com>
>
> With correct POSIX handling, unquoted empty variables can turn the
> expression like
> test -n ${fdtfile}
> into
> test -n
>
> The POSIX handling for single argument `test` evaluates it as true,
> so the fallback initialization will be skipped unexpectedly.
> Quoting variable expansions in `test` expressions will always result in
> correct behavior for empty and non-empty values.
> This change was triggered by
> commit 8b0619579b22 ("cmd: test: fix handling of single-argument form of test")
> The aim is to have a less fragile codebase that is not dependent on a
> quirk of the shell implementation.
>
> Use quoted variable expansions in `test` expressions throughout.
>
> Signed-off-by: Franz Schnyder <franz.schnyder@toradex.com>
Acked-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] board: toradex: Quote variables in `test` cmd expression
2026-04-01 5:10 ` Francesco Dolcini
@ 2026-04-02 23:30 ` Tom Rini
2026-04-03 8:53 ` Francesco Dolcini
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2026-04-02 23:30 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Franz Schnyder, u-boot, Francesco Dolcini, Rasmus Villemoes,
Franz Schnyder
[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]
On Wed, Apr 01, 2026 at 07:10:27AM +0200, Francesco Dolcini wrote:
> On Tue, Mar 31, 2026 at 10:10:10AM +0200, Franz Schnyder wrote:
> > From: Franz Schnyder <franz.schnyder@toradex.com>
> >
> > With correct POSIX handling, unquoted empty variables can turn the
> > expression like
> > test -n ${fdtfile}
> > into
> > test -n
> >
> > The POSIX handling for single argument `test` evaluates it as true,
> > so the fallback initialization will be skipped unexpectedly.
> > Quoting variable expansions in `test` expressions will always result in
> > correct behavior for empty and non-empty values.
> > This change was triggered by
> > commit 8b0619579b22 ("cmd: test: fix handling of single-argument form of test")
> > The aim is to have a less fragile codebase that is not dependent on a
> > quirk of the shell implementation.
> >
> > Use quoted variable expansions in `test` expressions throughout.
> >
> > Signed-off-by: Franz Schnyder <franz.schnyder@toradex.com>
>
> Acked-by: Francesco Dolcini <francesco.dolcini@toradex.com>
I've decided to go with:
https://patchwork.ozlabs.org/project/uboot/patch/20260330140106.401876-1-ravi@prevas.dk/
to make sure that everyone that might hit this problem doesn't end up
broken, thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] board: toradex: Quote variables in `test` cmd expression
2026-04-02 23:30 ` Tom Rini
@ 2026-04-03 8:53 ` Francesco Dolcini
2026-04-09 9:07 ` Rasmus Villemoes
0 siblings, 1 reply; 5+ messages in thread
From: Francesco Dolcini @ 2026-04-03 8:53 UTC (permalink / raw)
To: Tom Rini
Cc: Francesco Dolcini, Franz Schnyder, u-boot, Francesco Dolcini,
Rasmus Villemoes, Franz Schnyder
On Thu, Apr 02, 2026 at 05:30:29PM -0600, Tom Rini wrote:
> On Wed, Apr 01, 2026 at 07:10:27AM +0200, Francesco Dolcini wrote:
> > On Tue, Mar 31, 2026 at 10:10:10AM +0200, Franz Schnyder wrote:
> > > From: Franz Schnyder <franz.schnyder@toradex.com>
> > >
> > > With correct POSIX handling, unquoted empty variables can turn the
> > > expression like
> > > test -n ${fdtfile}
> > > into
> > > test -n
> > >
> > > The POSIX handling for single argument `test` evaluates it as true,
> > > so the fallback initialization will be skipped unexpectedly.
> > > Quoting variable expansions in `test` expressions will always result in
> > > correct behavior for empty and non-empty values.
> > > This change was triggered by
> > > commit 8b0619579b22 ("cmd: test: fix handling of single-argument form of test")
> > > The aim is to have a less fragile codebase that is not dependent on a
> > > quirk of the shell implementation.
> > >
> > > Use quoted variable expansions in `test` expressions throughout.
> > >
> > > Signed-off-by: Franz Schnyder <franz.schnyder@toradex.com>
> >
> > Acked-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>
> I've decided to go with:
> https://patchwork.ozlabs.org/project/uboot/patch/20260330140106.401876-1-ravi@prevas.dk/
> to make sure that everyone that might hit this problem doesn't end up
> broken, thanks.
I am wondering if you should apply this in any case. This change is generally correct.
In the past when trying to move to some more modern shell as default stuff broke
for us, we reverted the change without investigating. Maybe it was exactly the
same issue.
See https://lore.kernel.org/u-boot/20240111170418.GA7220@francesco-nb/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] board: toradex: Quote variables in `test` cmd expression
2026-04-03 8:53 ` Francesco Dolcini
@ 2026-04-09 9:07 ` Rasmus Villemoes
0 siblings, 0 replies; 5+ messages in thread
From: Rasmus Villemoes @ 2026-04-09 9:07 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Tom Rini, Franz Schnyder, u-boot, Francesco Dolcini,
Franz Schnyder
On Fri, Apr 03 2026, Francesco Dolcini <francesco@dolcini.it> wrote:
> On Thu, Apr 02, 2026 at 05:30:29PM -0600, Tom Rini wrote:
>> On Wed, Apr 01, 2026 at 07:10:27AM +0200, Francesco Dolcini wrote:
>> > On Tue, Mar 31, 2026 at 10:10:10AM +0200, Franz Schnyder wrote:
>> > > From: Franz Schnyder <franz.schnyder@toradex.com>
>> > >
>> > > With correct POSIX handling, unquoted empty variables can turn the
>> > > expression like
>> > > test -n ${fdtfile}
>> > > into
>> > > test -n
>> > >
>> > > The POSIX handling for single argument `test` evaluates it as true,
>> > > so the fallback initialization will be skipped unexpectedly.
>> > > Quoting variable expansions in `test` expressions will always result in
>> > > correct behavior for empty and non-empty values.
>> > > This change was triggered by
>> > > commit 8b0619579b22 ("cmd: test: fix handling of single-argument form of test")
>> > > The aim is to have a less fragile codebase that is not dependent on a
>> > > quirk of the shell implementation.
>> > >
>> > > Use quoted variable expansions in `test` expressions throughout.
>> > >
>> > > Signed-off-by: Franz Schnyder <franz.schnyder@toradex.com>
>> >
>> > Acked-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>
>> I've decided to go with:
>> https://patchwork.ozlabs.org/project/uboot/patch/20260330140106.401876-1-ravi@prevas.dk/
>> to make sure that everyone that might hit this problem doesn't end up
>> broken, thanks.
>
> I am wondering if you should apply this in any case. This change is generally correct.
I agree. And note that this really isn't anything to do with the 'test'
command per se, it's simply that using any variable whose value can be
empty or contain spaces is prone to produce something other than exactly
1 word, exactly as in userspace shell, and once the shell has done its
parsing and word splitting, the called command really has no way of
knowing that there's an empty argument missing.
> In the past when trying to move to some more modern shell as default stuff broke
> for us, we reverted the change without investigating. Maybe it was exactly the
> same issue.
>
> See https://lore.kernel.org/u-boot/20240111170418.GA7220@francesco-nb/
I don't think that was the same issue, but there are certainly differences
between the old and new parser when it comes to quoting and escaping,
and that 'env set set_apply_overlays ...' line looks quite prone to
being interpreted differently.
Rasmus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-09 9:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 8:10 [PATCH] board: toradex: Quote variables in `test` cmd expression Franz Schnyder
2026-04-01 5:10 ` Francesco Dolcini
2026-04-02 23:30 ` Tom Rini
2026-04-03 8:53 ` Francesco Dolcini
2026-04-09 9:07 ` Rasmus Villemoes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox