On Fri, Jan 16, 2026 at 10:22:13AM +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <[email protected]>In Bash, the following code does not do what you think it does: func() { local var=$(false) echo $? } Here, '0' is echoed even though false is designed to exit with a non-zero code. This is because in fact the last executed command is 'local' not 'false' and thus '$?' contains zero as in "yeah, the variable is successfully declared" [1]. In our libvirt-guest shell script, there are a few places like this. Now, in next commit a syntax-check rule is introduced and to keep it simple (by avoiding multi line grep) it'll deny declaring local variables and assigning their value via a subshell regardless of subsequent '$?' check. Thus, this commit may change more than necessary, strictly speaking. But in the long run, we're on the safe side. 1: https://www.shellcheck.net/wiki/SC2155 Fixes: 08071ec0f113bb1fe8dcc263cb6bf87529e8b76b Resolves: https://gitlab.com/libvirt/libvirt/-/issues/839 Signed-off-by: Michal Privoznik <[email protected]> --- tools/libvirt-guests.sh.in | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index f2db1282ad..f57eda66fe 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -106,7 +106,9 @@ test_connect() list_guests() { local uri="$1" local persistent="$2" - local list="$(run_virsh_c "$uri" list --uuid $persistent)" + local list=
You don't need the equals sign here, just:
local list
list="$(...)"
also everywhere below.
[...]
@@ -226,11 +230,13 @@ suspend_guest()
{
local uri="$1"
local guest="$2"
- local name="$(guest_name "$uri" "$guest")"
- local label="$(eval_gettext "Suspending \$name: ")"
+ local name=
+ local label=
local bypass=
local slept=0
here you can just remove all `^local ` and start the function with:
local uri guest name label bypass slept progress #(from below) etc.
it would be nicer, I think. If you changed it with a script you could
modify it for all functions, but if you did it manually, then it's a
`good-first-issue` candidate, I guess.
signature.asc
Description: PGP signature
