| Message ID | 20240405090052.375599-8-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add shellcheck support | expand |
On Fri, Apr 05, 2024 at 07:00:39PM +1000, Nicholas Piggin wrote: > SC2235 (style): Use { ..; } instead of (..) to avoid subshell > overhead. > > No bug identified. Overhead is pretty irrelevant. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > scripts/arch-run.bash | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > index ae4b06679..d1edd1d69 100644 > --- a/scripts/arch-run.bash > +++ b/scripts/arch-run.bash > @@ -580,15 +580,15 @@ kvm_available () > return 1 > > [ "$HOST" = "$ARCH_NAME" ] || > - ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > + { [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ; } || > + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } > } > > hvf_available () > { > [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 > [ "$HOST" = "$ARCH_NAME" ] || > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } > } This one is a bit ugly. Most developers are used to seeing expressions like a || (b && c) not a || { b && c ; } I'm inclined to add SC2235 to the ignore list... Thanks, drew
On Sat Apr 6, 2024 at 12:24 AM AEST, Andrew Jones wrote: > On Fri, Apr 05, 2024 at 07:00:39PM +1000, Nicholas Piggin wrote: > > SC2235 (style): Use { ..; } instead of (..) to avoid subshell > > overhead. > > > > No bug identified. Overhead is pretty irrelevant. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > scripts/arch-run.bash | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > index ae4b06679..d1edd1d69 100644 > > --- a/scripts/arch-run.bash > > +++ b/scripts/arch-run.bash > > @@ -580,15 +580,15 @@ kvm_available () > > return 1 > > > > [ "$HOST" = "$ARCH_NAME" ] || > > - ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || > > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > > + { [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ; } || > > + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } > > } > > > > hvf_available () > > { > > [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 > > [ "$HOST" = "$ARCH_NAME" ] || > > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > > + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } > > } > > This one is a bit ugly. Most developers are used to seeing expressions > like > > a || (b && c) > > not > > a || { b && c ; } > > I'm inclined to add SC2235 to the ignore list... I'm sure it's a good warning for bash scripts that do a lot of computation and have these in hot loops. But I _think_ none of our scripts are like that so I would be okay with disabling the warning. I'll do that unless someone disagrees. Thanks, Nick
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index ae4b06679..d1edd1d69 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -580,15 +580,15 @@ kvm_available () return 1 [ "$HOST" = "$ARCH_NAME" ] || - ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) + { [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ; } || + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } } hvf_available () { [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 [ "$HOST" = "$ARCH_NAME" ] || - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) + { [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ; } } set_qemu_accelerator ()
SC2235 (style): Use { ..; } instead of (..) to avoid subshell overhead. No bug identified. Overhead is pretty irrelevant. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- scripts/arch-run.bash | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)