diff mbox series

[2/5] configure: Add braces to clarify intent of $emu[[:space:]]

Message ID 20220720152631.450903-3-peter.maydell@linaro.org
State Superseded
Headers show
Series configure: fix some non-portabilities | expand

Commit Message

Peter Maydell July 20, 2022, 3:26 p.m. UTC
In shell script syntax, $var[something] is not special for variable
expansion: $emu is expanded.  However, as it can look as if it were
intended to be an array element access (the correct syntax for which
is ${var[something]}), shellcheck recommends using explicit braces
around ${var} to clarify the intended expansion.

This fixes the warning:

In ./configure line 2346:
        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
                                                              ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is our only 'error' level shellcheck warning, so it seems
worth suppressing even though it's not wrong as written.
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Huth July 20, 2022, 3:36 p.m. UTC | #1
On 20/07/2022 17.26, Peter Maydell wrote:
> In shell script syntax, $var[something] is not special for variable
> expansion: $emu is expanded.  However, as it can look as if it were
> intended to be an array element access (the correct syntax for which
> is ${var[something]}), shellcheck recommends using explicit braces
> around ${var} to clarify the intended expansion.
> 
> This fixes the warning:
> 
> In ./configure line 2346:
>          if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
>                                                                ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is our only 'error' level shellcheck warning, so it seems
> worth suppressing even though it's not wrong as written.
> ---
>   configure | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index dec6f030346..a56c3d921be 100755
> --- a/configure
> +++ b/configure
> @@ -2343,7 +2343,7 @@ if test -n "$target_cc" &&
>       # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
>       # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
>       for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
> -        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
> +        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
>               ld_i386_emulation="$emu"
>               break
>           fi

Reviewed-by: Thomas Huth <thuth@redhat.com>
Markus Armbruster July 21, 2022, 6:24 a.m. UTC | #2
Peter Maydell <peter.maydell@linaro.org> writes:

> In shell script syntax, $var[something] is not special for variable
> expansion: $emu is expanded.  However, as it can look as if it were

Do you mean "$var is expanded"?

> intended to be an array element access (the correct syntax for which
> is ${var[something]}), shellcheck recommends using explicit braces
> around ${var} to clarify the intended expansion.
>
> This fixes the warning:
>
> In ./configure line 2346:
>         if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
>                                                               ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is our only 'error' level shellcheck warning, so it seems
> worth suppressing even though it's not wrong as written.
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index dec6f030346..a56c3d921be 100755
> --- a/configure
> +++ b/configure
> @@ -2343,7 +2343,7 @@ if test -n "$target_cc" &&
>      # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
>      # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
>      for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
> -        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
> +        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
>              ld_i386_emulation="$emu"
>              break
>          fi
Peter Maydell July 21, 2022, 8:28 a.m. UTC | #3
On Thu, 21 Jul 2022 at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > In shell script syntax, $var[something] is not special for variable
> > expansion: $emu is expanded.  However, as it can look as if it were
>
> Do you mean "$var is expanded"?

Yes. I changed my mind half way through writing the commit message
about whether to use as my example the specific variable name we
have in configure or a generic $var...

-- PMM
diff mbox series

Patch

diff --git a/configure b/configure
index dec6f030346..a56c3d921be 100755
--- a/configure
+++ b/configure
@@ -2343,7 +2343,7 @@  if test -n "$target_cc" &&
     # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
     # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
     for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
-        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
+        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
             ld_i386_emulation="$emu"
             break
         fi