[PATCH 0/4] Minor improvements to scripts/run-qemu.sh

[PATCH 0/4] Minor improvements to scripts/run-qemu.sh

[Demi Marie Obenour]

None of these are major changes.  See individual commit messages for
details.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
Demi Marie Obenour (4):
      scripts/run-qemu.sh: better error checks & formatting
      scripts/run-qemu.sh: decrease indentation
      scripts/run-qemu.sh: Preserve ,, in QEMU arguments
      scripts/run-qemu.sh: Unset variables from the environment

 scripts/run-qemu.sh | 123 +++++++++++++++++++++++++++-------------------------
 1 file changed, 65 insertions(+), 58 deletions(-)
---
base-commit: 22e216712322cdfb85094bbd27ff34c4366fad41
change-id: 20251107-simple-sh-quote-fix-cf956d5422c3

-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Demi Marie Obenour]

No other functional change intended.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/run-qemu.sh | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
--- a/scripts/run-qemu.sh
+++ b/scripts/run-qemu.sh
@@ -1,11 +1,15 @@
-#!/bin/sh -ue
+#!/bin/sh --
 # SPDX-FileCopyrightText: 2023-2025 Alyssa Ross <hi@alyssa.is>
 # SPDX-License-Identifier: EUPL-1.2+
 
 # This script wraps around QEMU to paper over platform differences,
 # which can't be handled portably in Make language.
+set -uef
+if [ -n ${ARCH+test} ]; then
+	ARCH=$(uname -m)
+fi
 
-case "${ARCH:="$(uname -m)"}" in
+case $ARCH in
 	aarch64)
 		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
 		;;
@@ -17,13 +21,13 @@ case "${ARCH:="$(uname -m)"}" in
 esac
 
 i=0
-while [ $i -lt $# ]; do
-	arg="$1"
+while [ "$i" -lt "$#" ]; do
+	arg=$1
 	shift
 
-	case "$arg" in
+	case $arg in
 		-append)
-			set -- "$@" -append "${append:+$append }$1"
+			set -- "$@" -append ${append:+"$append "}"$1"
 			i=$((i + 2))
 			shift
 			continue
@@ -31,10 +35,10 @@ while [ $i -lt $# ]; do
 		-device)
 			IFS=,
 			for opt in $1; do
-				case "$opt" in
-					*-iommu)
-						unset iommu
-						;;
+				case $opt in
+				*-iommu)
+					unset iommu
+					;;
 				esac
 				break
 			done

-- 
2.51.2

[PATCH 2/4] scripts/run-qemu.sh: decrease indentation

[Demi Marie Obenour]

The rightward drift made the code harder to read.

No functional change.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/run-qemu.sh | 106 ++++++++++++++++++++++++++--------------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
index 9c6c8193bbeba5916038c82d8f76992051719c19..ab324d8fda0810516cb2180d1372bbeb546f52bc 100755
--- a/scripts/run-qemu.sh
+++ b/scripts/run-qemu.sh
@@ -10,14 +10,14 @@ if [ -n ${ARCH+test} ]; then
 fi
 
 case $ARCH in
-	aarch64)
-		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
-		;;
-	x86_64)
-		append="console=ttyS0${append:+ $append}"
-		iommu=intel-iommu,intremap=on
-		machine=q35,accel=kvm:tcg,kernel-irqchip=split
-		;;
+aarch64)
+	machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
+	;;
+x86_64)
+	append="console=ttyS0${append:+ $append}"
+	iommu=intel-iommu,intremap=on
+	machine=q35,accel=kvm:tcg,kernel-irqchip=split
+	;;
 esac
 
 i=0
@@ -26,49 +26,49 @@ while [ "$i" -lt "$#" ]; do
 	shift
 
 	case $arg in
-		-append)
-			set -- "$@" -append ${append:+"$append "}"$1"
-			i=$((i + 2))
-			shift
-			continue
-			;;
-		-device)
-			IFS=,
-			for opt in $1; do
-				case $opt in
-				*-iommu)
-					unset iommu
-					;;
-				esac
-				break
-			done
-			unset IFS
-			;;
-		-machine)
-			set -- "$@" "$arg"
-			i=$((i + 1))
-			arg=
+	-append)
+		set -- "$@" -append ${append:+"$append "}"$1"
+		i=$((i + 2))
+		shift
+		continue
+		;;
+	-device)
+		IFS=,
+		for opt in $1; do
+			case $opt in
+			*-iommu)
+				unset iommu
+				;;
+			esac
+			break
+		done
+		unset IFS
+		;;
+	-machine)
+		set -- "$@" "$arg"
+		i=$((i + 1))
+		arg=
 
-			IFS=,
-			for opt in $1; do
-				case "$opt" in
-					virtualization=on)
-						case "$ARCH" in
-							aarch64)
-								opt="$opt,accel=tcg"
-								;;
-							*)
-								continue
-								;;
-						esac
-						;;
+		IFS=,
+		for opt in $1; do
+			case "$opt" in
+			virtualization=on)
+				case "$ARCH" in
+				aarch64)
+					opt="$opt,accel=tcg"
+					;;
+				*)
+					continue
+					;;
 				esac
+				;;
+			esac
 
-				arg="$arg${arg:+,}$opt"
-			done
-			unset IFS
+			arg="$arg${arg:+,}$opt"
+		done
+		unset IFS
 
-			shift
+		shift
 	esac
 
 	set -- "$@" "$arg"
@@ -78,12 +78,12 @@ done
 
 for arg; do
 	case "$arg" in
-		-append)
-			append=
-			;;
-		-kernel)
-			kernel=1
-			;;
+	-append)
+		append=
+		;;
+	-kernel)
+		kernel=1
+		;;
 	esac
 done
 

-- 
2.51.2

[PATCH 3/4] scripts/run-qemu.sh: Preserve ,, in QEMU arguments

[Demi Marie Obenour]

QEMU allows commas in option values to be escaped by doubling them.
Therefore, doubled commas should be preserved by scripts/run-qemu.sh.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/run-qemu.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
index ab324d8fda0810516cb2180d1372bbeb546f52bc..14c58729dfceb3a0d2566fcfe076d6c10433419f 100755
--- a/scripts/run-qemu.sh
+++ b/scripts/run-qemu.sh
@@ -64,7 +64,7 @@ while [ "$i" -lt "$#" ]; do
 				;;
 			esac
 
-			arg="$arg${arg:+,}$opt"
+			arg="$arg,$opt"
 		done
 		unset IFS
 

-- 
2.51.2

[PATCH 4/4] scripts/run-qemu.sh: Unset variables from the environment

[Demi Marie Obenour]

These could affect the script.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/run-qemu.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
index 14c58729dfceb3a0d2566fcfe076d6c10433419f..ba32467956a509b1744ade461f2fe5f20ae7a5b9 100755
--- a/scripts/run-qemu.sh
+++ b/scripts/run-qemu.sh
@@ -9,12 +9,15 @@ if [ -n ${ARCH+test} ]; then
 	ARCH=$(uname -m)
 fi
 
+unset kernel
+
 case $ARCH in
 aarch64)
 	machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
+	unset iommu append
 	;;
 x86_64)
-	append="console=ttyS0${append:+ $append}"
+	append=console=ttyS0
 	iommu=intel-iommu,intremap=on
 	machine=q35,accel=kvm:tcg,kernel-irqchip=split
 	;;
@@ -79,7 +82,7 @@ done
 for arg; do
 	case "$arg" in
 	-append)
-		append=
+		unset append
 		;;
 	-kernel)
 		kernel=1

-- 
2.51.2

Re: [PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> No other functional change intended.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  scripts/run-qemu.sh | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

If we're doing formatting changes like this it's definitely time to
adopt a shell script formatter, so I'm going to apply my previous series
for that[1].

If you can find a shell formatter that does matched parentheses for case
statements, we can switch. :)

[1]: https://spectrum-os.org/lists/archives/spectrum-devel/20251001102838.7086-1-hi@alyssa.is

> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
> index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
> --- a/scripts/run-qemu.sh
> +++ b/scripts/run-qemu.sh
> @@ -1,11 +1,15 @@
> -#!/bin/sh -ue
> +#!/bin/sh --
>  # SPDX-FileCopyrightText: 2023-2025 Alyssa Ross <hi@alyssa.is>
>  # SPDX-License-Identifier: EUPL-1.2+
>  
>  # This script wraps around QEMU to paper over platform differences,
>  # which can't be handled portably in Make language.
> +set -uef
> +if [ -n ${ARCH+test} ]; then
> +	ARCH=$(uname -m)
> +fi
>  
> -case "${ARCH:="$(uname -m)"}" in
> +case $ARCH in
>  	aarch64)
>  		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
>  		;;
> @@ -17,13 +21,13 @@ case "${ARCH:="$(uname -m)"}" in
>  esac
>  
>  i=0
> -while [ $i -lt $# ]; do
> -	arg="$1"
> +while [ "$i" -lt "$#" ]; do
> +	arg=$1
>  	shift
>  
> -	case "$arg" in
> +	case $arg in
>  		-append)
> -			set -- "$@" -append "${append:+$append }$1"
> +			set -- "$@" -append ${append:+"$append "}"$1"
>  			i=$((i + 2))
>  			shift
>  			continue
> @@ -31,10 +35,10 @@ while [ $i -lt $# ]; do
>  		-device)
>  			IFS=,
>  			for opt in $1; do
> -				case "$opt" in
> -					*-iommu)
> -						unset iommu
> -						;;
> +				case $opt in
> +				*-iommu)
> +					unset iommu
> +					;;
>  				esac
>  				break
>  			done
>
> -- 
> 2.51.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> No other functional change intended.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  scripts/run-qemu.sh | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

This is a lot of changes all in one, without individual explanation.
I'd appreciate it if they could be broken up and explained, because I'm
going to have to ask what the purpose of each change is.

> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
> index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
> --- a/scripts/run-qemu.sh
> +++ b/scripts/run-qemu.sh
> @@ -1,11 +1,15 @@
> -#!/bin/sh -ue
> +#!/bin/sh --

The idea here is to be robust against the script being invoked with
argv[0] set to "-c" or something?

>  # SPDX-FileCopyrightText: 2023-2025 Alyssa Ross <hi@alyssa.is>
>  # SPDX-License-Identifier: EUPL-1.2+
>  
>  # This script wraps around QEMU to paper over platform differences,
>  # which can't be handled portably in Make language.
> +set -uef

Adding -f makes sense.

> +if [ -n ${ARCH+test} ]; then
> +	ARCH=$(uname -m)
> +fi
>  
> -case "${ARCH:="$(uname -m)"}" in
> +case $ARCH in
>  	aarch64)
>  		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
>  		;;

Why is this better?

> @@ -17,13 +21,13 @@ case "${ARCH:="$(uname -m)"}" in
>  esac
>  
>  i=0
> -while [ $i -lt $# ]; do
> -	arg="$1"
> +while [ "$i" -lt "$#" ]; do
> +	arg=$1
>  	shift
>  
> -	case "$arg" in
> +	case $arg in
>  		-append)

Makes sense as long as we're consistent about it.  I wonder if we could
get the formatter to do this.

> -			set -- "$@" -append "${append:+$append }$1"
> +			set -- "$@" -append ${append:+"$append "}"$1"
>  			i=$((i + 2))
>  			shift
>  			continue

Don't understand this one.  We've gone from one set of quotes to two
sequential ones.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 4/4] scripts/run-qemu.sh: Unset variables from the environment

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> These could affect the script.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  scripts/run-qemu.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Makes sense.

> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
> index 14c58729dfceb3a0d2566fcfe076d6c10433419f..ba32467956a509b1744ade461f2fe5f20ae7a5b9 100755
> --- a/scripts/run-qemu.sh
> +++ b/scripts/run-qemu.sh
> @@ -9,12 +9,15 @@ if [ -n ${ARCH+test} ]; then
>  	ARCH=$(uname -m)
>  fi
>  
> +unset kernel
> +

Would be clearer if this was attached to the for loop that actually sets
kernel, I think.  Then as I reader I don't need to keep it in my head
for as long.  (I can make this change and commit if you agree.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 3/4] scripts/run-qemu.sh: Preserve ,, in QEMU arguments

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> QEMU allows commas in option values to be escaped by doubling them.
> Therefore, doubled commas should be preserved by scripts/run-qemu.sh.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  scripts/run-qemu.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This makes all the tests fail:

qemu-system-aarch64: -machine ,virtualization=on,accel=tcg: Invalid parameter ''

> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
> index ab324d8fda0810516cb2180d1372bbeb546f52bc..14c58729dfceb3a0d2566fcfe076d6c10433419f 100755
> --- a/scripts/run-qemu.sh
> +++ b/scripts/run-qemu.sh
> @@ -64,7 +64,7 @@ while [ "$i" -lt "$#" ]; do
>  				;;
>  			esac
>  
> -			arg="$arg${arg:+,}$opt"
> +			arg="$arg,$opt"
>  		done
>  		unset IFS
>  
>
> -- 
> 2.51.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 2345 bytes --]

On 11/13/25 07:23, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> No other functional change intended.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  scripts/run-qemu.sh | 24 ++++++++++++++----------
>>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> This is a lot of changes all in one, without individual explanation.
> I'd appreciate it if they could be broken up and explained, because I'm
> going to have to ask what the purpose of each change is.
> 
>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>> index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
>> --- a/scripts/run-qemu.sh
>> +++ b/scripts/run-qemu.sh
>> @@ -1,11 +1,15 @@
>> -#!/bin/sh -ue
>> +#!/bin/sh --
> 
> The idea here is to be robust against the script being invoked with
> argv[0] set to "-c" or something?

Correct.

>>  # SPDX-FileCopyrightText: 2023-2025 Alyssa Ross <hi@alyssa.is>
>>  # SPDX-License-Identifier: EUPL-1.2+
>>  
>>  # This script wraps around QEMU to paper over platform differences,
>>  # which can't be handled portably in Make language.
>> +set -uef
> 
> Adding -f makes sense.
> 
>> +if [ -n ${ARCH+test} ]; then
>> +	ARCH=$(uname -m)
>> +fi
>>  
>> -case "${ARCH:="$(uname -m)"}" in
>> +case $ARCH in
>>  	aarch64)
>>  		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
>>  		;;
> 
> Why is this better?

If uname exits with a non-zero status, the script will exit rather
than continuing.

>> @@ -17,13 +21,13 @@ case "${ARCH:="$(uname -m)"}" in
>>  esac
>>  
>>  i=0
>> -while [ $i -lt $# ]; do
>> -	arg="$1"
>> +while [ "$i" -lt "$#" ]; do
>> +	arg=$1
>>  	shift
>>  
>> -	case "$arg" in
>> +	case $arg in
>>  		-append)
> 
> Makes sense as long as we're consistent about it.  I wonder if we could
> get the formatter to do this.

+1

>> -			set -- "$@" -append "${append:+$append }$1"
>> +			set -- "$@" -append ${append:+"$append "}"$1"
>>  			i=$((i + 2))
>>  			shift
>>  			continue
> 
> Don't understand this one.  We've gone from one set of quotes to two
> sequential ones.

I remember reading that the first version might not conform to POSIX.
I'm not sure if it matters though.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] scripts/run-qemu.sh: Preserve ,, in QEMU arguments

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 1019 bytes --]

On 11/13/25 07:28, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> QEMU allows commas in option values to be escaped by doubling them.
>> Therefore, doubled commas should be preserved by scripts/run-qemu.sh.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  scripts/run-qemu.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> This makes all the tests fail:
> 
> qemu-system-aarch64: -machine ,virtualization=on,accel=tcg: Invalid parameter ''

Whoops, just drop this.

>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>> index ab324d8fda0810516cb2180d1372bbeb546f52bc..14c58729dfceb3a0d2566fcfe076d6c10433419f 100755
>> --- a/scripts/run-qemu.sh
>> +++ b/scripts/run-qemu.sh
>> @@ -64,7 +64,7 @@ while [ "$i" -lt "$#" ]; do
>>  				;;
>>  			esac
>>  
>> -			arg="$arg${arg:+,}$opt"
>> +			arg="$arg,$opt"
>>  		done
>>  		unset IFS
>>  
>>
>> -- 
>> 2.51.2
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 11/13/25 07:23, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>> 
>>> No other functional change intended.
>>>
>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>> ---
>>>  scripts/run-qemu.sh | 24 ++++++++++++++----------
>>>  1 file changed, 14 insertions(+), 10 deletions(-)
>> 
>> This is a lot of changes all in one, without individual explanation.
>> I'd appreciate it if they could be broken up and explained, because I'm
>> going to have to ask what the purpose of each change is.
>> 
>>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>>> index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
>>> --- a/scripts/run-qemu.sh
>>> +++ b/scripts/run-qemu.sh
>>> @@ -1,11 +1,15 @@
>>> -#!/bin/sh -ue
>>> +#!/bin/sh --
>> 
>> The idea here is to be robust against the script being invoked with
>> argv[0] set to "-c" or something?
>
> Correct.

Seems a little paranoid but I suppose it doesn't hurt.  I'd take a
treewide change that did this for every /bin/sh shebang.

>>> +if [ -n ${ARCH+test} ]; then
>>> +	ARCH=$(uname -m)
>>> +fi
>>>  
>>> -case "${ARCH:="$(uname -m)"}" in
>>> +case $ARCH in
>>>  	aarch64)
>>>  		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
>>>  		;;
>> 
>> Why is this better?
>
> If uname exits with a non-zero status, the script will exit rather
> than continuing.

I see — makes sense.  And why is the check not just the following?

	if [ -z "${ARCH-}" ]; then

>>> -			set -- "$@" -append "${append:+$append }$1"
>>> +			set -- "$@" -append ${append:+"$append "}"$1"
>>>  			i=$((i + 2))
>>>  			shift
>>>  			continue
>> 
>> Don't understand this one.  We've gone from one set of quotes to two
>> sequential ones.
>
> I remember reading that the first version might not conform to POSIX.
> I'm not sure if it matters though.

POSIX says[1]:

> If a parameter expansion occurs inside double-quotes:
> 
> * Pathname expansion shall not be performed on the results of the expansion.
> 
> * Field splitting shall not be performed on the results of the expansion.

Seems desirable to me.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/4] scripts/run-qemu.sh: better error checks & formatting

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 2751 bytes --]

On 11/13/25 11:56, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> On 11/13/25 07:23, Alyssa Ross wrote:
>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>
>>>> No other functional change intended.
>>>>
>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>> ---
>>>>  scripts/run-qemu.sh | 24 ++++++++++++++----------
>>>>  1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> This is a lot of changes all in one, without individual explanation.
>>> I'd appreciate it if they could be broken up and explained, because I'm
>>> going to have to ask what the purpose of each change is.
>>>
>>>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>>>> index 64fd29259ab108bc547cb7c74623ae9dc288b3b7..9c6c8193bbeba5916038c82d8f76992051719c19 100755
>>>> --- a/scripts/run-qemu.sh
>>>> +++ b/scripts/run-qemu.sh
>>>> @@ -1,11 +1,15 @@
>>>> -#!/bin/sh -ue
>>>> +#!/bin/sh --
>>>
>>> The idea here is to be robust against the script being invoked with
>>> argv[0] set to "-c" or something?
>>
>> Correct.
> 
> Seems a little paranoid but I suppose it doesn't hurt.  I'd take a
> treewide change that did this for every /bin/sh shebang.

Will do later.

>>>> +if [ -n ${ARCH+test} ]; then
>>>> +	ARCH=$(uname -m)
>>>> +fi
>>>>  
>>>> -case "${ARCH:="$(uname -m)"}" in
>>>> +case $ARCH in
>>>>  	aarch64)
>>>>  		machine=virt,accel=kvm:tcg,gic-version=3,iommu=smmuv3
>>>>  		;;
>>>
>>> Why is this better?
>>
>> If uname exits with a non-zero status, the script will exit rather
>> than continuing.
> 
> I see — makes sense.  And why is the check not just the following?
> 
> 	if [ -z "${ARCH-}" ]; then

That works just as well.

>>>> -			set -- "$@" -append "${append:+$append }$1"
>>>> +			set -- "$@" -append ${append:+"$append "}"$1"
>>>>  			i=$((i + 2))
>>>>  			shift
>>>>  			continue
>>>
>>> Don't understand this one.  We've gone from one set of quotes to two
>>> sequential ones.
>>
>> I remember reading that the first version might not conform to POSIX.
>> I'm not sure if it matters though.
> 
> POSIX says[1]:
> 
>> If a parameter expansion occurs inside double-quotes:
>>
>> * Pathname expansion shall not be performed on the results of the expansion.
>>
>> * Field splitting shall not be performed on the results of the expansion.
> 
> Seems desirable to me.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

I recall reading about shells not properly handling
${append:+"$append"} properly when the whole expansion is itself quoted
The double quotes around "$append" prevent it from being expanded.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] scripts/run-qemu.sh: Preserve ,, in QEMU arguments

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 1033 bytes --]

On 11/13/25 07:28, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> QEMU allows commas in option values to be escaped by doubling them.
>> Therefore, doubled commas should be preserved by scripts/run-qemu.sh.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  scripts/run-qemu.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> This makes all the tests fail:
> 
> qemu-system-aarch64: -machine ,virtualization=on,accel=tcg: Invalid parameter ''
> 
>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>> index ab324d8fda0810516cb2180d1372bbeb546f52bc..14c58729dfceb3a0d2566fcfe076d6c10433419f 100755
>> --- a/scripts/run-qemu.sh
>> +++ b/scripts/run-qemu.sh
>> @@ -64,7 +64,7 @@ while [ "$i" -lt "$#" ]; do
>>  				;;
>>  			esac
>>  
>> -			arg="$arg${arg:+,}$opt"
>> +			arg="$arg,$opt"
>>  		done
>>  		unset IFS
>>  
>>
>> -- 
>> 2.51.2

This was just wrong.  I'll drop it.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] scripts/run-qemu.sh: Unset variables from the environment

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 955 bytes --]

On 11/13/25 07:27, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> These could affect the script.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  scripts/run-qemu.sh | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Makes sense.
> 
>> diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
>> index 14c58729dfceb3a0d2566fcfe076d6c10433419f..ba32467956a509b1744ade461f2fe5f20ae7a5b9 100755
>> --- a/scripts/run-qemu.sh
>> +++ b/scripts/run-qemu.sh
>> @@ -9,12 +9,15 @@ if [ -n ${ARCH+test} ]; then
>>  	ARCH=$(uname -m)
>>  fi
>>  
>> +unset kernel
>> +
> 
> Would be clearer if this was attached to the for loop that actually sets
> kernel, I think.  Then as I reader I don't need to keep it in my head
> for as long.  (I can make this change and commit if you agree.)

Feel free to fix up on commit.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] scripts/run-qemu.sh: Unset variables from the environment

[Alyssa Ross]

This patch has been committed as 26c6622abe98797d5d3417300b4473fd119d6d25,
which can be viewed online at
https://spectrum-os.org/git/spectrum/commit/?id=26c6622abe98797d5d3417300b4473fd119d6d25.

This is an automated message.  Send comments/questions/requests to:
Alyssa Ross <hi@alyssa.is>