On Fri, Jan 16, 2026 at 10:22:13AM +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
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 <mprivozn@redhat.com> --- 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.