All the tests are prefixed with 'x', in order to avoid risky comparisons
(i.e. a user deliberately trying to provoke a syntax error).

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index c728443ba2..98401f4e7c 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -259,10 +259,10 @@ qemu_check_systemd() {
 
 qemu_generate_register() {
     flags=""
-    if [ "$CREDENTIAL" = "yes" ]; then
+    if [ "x$CREDENTIAL" = "xyes" ]; then
         flags="OC"
     fi
-    if [ "$PERSISTENT" = "yes" ]; then
+    if [ "x$PERSISTENT" = "xyes" ]; then
         flags="${flags}F"
     fi
 
@@ -300,18 +300,18 @@ qemu_set_binfmts() {
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
 
-        if [ "$magic" = "" ] || [ "$mask" = "" ] || [ "$family" = "" ]; then
+        if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ]; then
             echo "INTERNAL ERROR: unknown cpu $cpu" 1>&2
             continue
         fi
 
         qemu="$QEMU_PATH/qemu-$cpu"
-        if [ "$cpu" = "i486" ]; then
+        if [ "x$cpu" = "xi486" ]; then
             qemu="$QEMU_PATH/qemu-i386"
         fi
 
         qemu="$qemu$QEMU_SUFFIX"
-        if [ "$host_family" != "$family" ]; then
+        if [ "x$host_family" != "x$family" ]; then
             $BINFMT_SET
         fi
     done
-- 
2.25.1

On 3/9/20 2:19 PM, Unai Martinez-Corral wrote:
> All the tests are prefixed with 'x', in order to avoid risky comparisons
> (i.e. a user deliberately trying to provoke a syntax error).
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   scripts/qemu-binfmt-conf.sh | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Le 09/03/2020 à 20:19, Unai Martinez-Corral a écrit :
> All the tests are prefixed with 'x', in order to avoid risky comparisons
> (i.e. a user deliberately trying to provoke a syntax error).

With the quotes I don't see how we can provoke a syntax error.
Could you provide an example?

Thanks,
Laurent

> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  scripts/qemu-binfmt-conf.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index c728443ba2..98401f4e7c 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -259,10 +259,10 @@ qemu_check_systemd() {
>  
>  qemu_generate_register() {
>      flags=""
> -    if [ "$CREDENTIAL" = "yes" ]; then
> +    if [ "x$CREDENTIAL" = "xyes" ]; then
>          flags="OC"
>      fi
> -    if [ "$PERSISTENT" = "yes" ]; then
> +    if [ "x$PERSISTENT" = "xyes" ]; then
>          flags="${flags}F"
>      fi
>  
> @@ -300,18 +300,18 @@ qemu_set_binfmts() {
>          mask=$(eval echo \$${cpu}_mask)
>          family=$(eval echo \$${cpu}_family)
>  
> -        if [ "$magic" = "" ] || [ "$mask" = "" ] || [ "$family" = "" ]; then
> +        if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ]; then
>              echo "INTERNAL ERROR: unknown cpu $cpu" 1>&2
>              continue
>          fi
>  
>          qemu="$QEMU_PATH/qemu-$cpu"
> -        if [ "$cpu" = "i486" ]; then
> +        if [ "x$cpu" = "xi486" ]; then
>              qemu="$QEMU_PATH/qemu-i386"
>          fi
>  
>          qemu="$qemu$QEMU_SUFFIX"
> -        if [ "$host_family" != "$family" ]; then
> +        if [ "x$host_family" != "x$family" ]; then
>              $BINFMT_SET
>          fi
>      done
> 

On 3/10/20 3:28 AM, Laurent Vivier wrote:
> Le 09/03/2020 à 20:19, Unai Martinez-Corral a écrit :
>> All the tests are prefixed with 'x', in order to avoid risky comparisons
>> (i.e. a user deliberately trying to provoke a syntax error).
> 
> With the quotes I don't see how we can provoke a syntax error.
> Could you provide an example?

Historically, in some shells:

foo=\(
bar=\)
if [ "$foo" = "$bar" ]; then echo hello world; fi

could output 'hello world' (by parsing a parenthesized one-argument 
test, and the string '=' is non-empty), but:

if [ "x$foo" = "x$bar" ]; then echo goodbye; fi

did not (since no operator begins with 'x', you have guaranteed the 
syntax that [ will parse).  Similarly, if foo=! or foo=-a, you could get 
syntax errors (if [ tried to treat the expansion of $foo as an operator 
and got thrown off by the remaining arguments not matching an expected 
pattern).

These days, POSIX says that with three arguments when the 2nd is a 
binary operator, there is no ambiguity (the binary operator takes 
precedence over the ( and ) around the non-empty string test), and 
modern bash obeys the POSIX rule without needing the x prefix.  But it 
is still better to prefix with x for copy-paste portability to older 
shells that do not match current POSIX rules.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Laurent, as Eric explained, this commit (as well as the homogeneisation of
spacing) are fixes to pre-existing issues in the script, in order to better
match other bash sources in the codebase. It would be possible to pick 1/10
and 2/10 as a separate patchset.

El mar., 10 mar. 2020 a las 12:47, Eric Blake (<eblake@redhat.com>)
escribió:

> On 3/10/20 3:28 AM, Laurent Vivier wrote:
> > Le 09/03/2020 à 20:19, Unai Martinez-Corral a écrit :
> >> All the tests are prefixed with 'x', in order to avoid risky comparisons
> >> (i.e. a user deliberately trying to provoke a syntax error).
> >
> > With the quotes I don't see how we can provoke a syntax error.
> > Could you provide an example?
>
> Historically, in some shells:
>
> foo=\(
> bar=\)
> if [ "$foo" = "$bar" ]; then echo hello world; fi
>
> could output 'hello world' (by parsing a parenthesized one-argument
> test, and the string '=' is non-empty), but:
>
> if [ "x$foo" = "x$bar" ]; then echo goodbye; fi
>
> did not (since no operator begins with 'x', you have guaranteed the
> syntax that [ will parse).  Similarly, if foo=! or foo=-a, you could get
> syntax errors (if [ tried to treat the expansion of $foo as an operator
> and got thrown off by the remaining arguments not matching an expected
> pattern).
>
> These days, POSIX says that with three arguments when the 2nd is a
> binary operator, there is no ambiguity (the binary operator takes
> precedence over the ( and ) around the non-empty string test), and
> modern bash obeys the POSIX rule without needing the x prefix.  But it
> is still better to prefix with x for copy-paste portability to older
> shells that do not match current POSIX rules.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
>