Re: [Qemu-devel] [PATCH] qemu-binfmt-conf.sh: add CPUS, add --reset, make -p and -c boolean (no arg)

[Eric Blake]

On 3/5/19 12:28 AM, Unai Martinez Corral wrote:
> Related to https://bugs.launchpad.net/qemu/+bug/1817239

> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -6,6 +6,32 @@ mips mipsel mipsn32 mipsn32el mips64 mips64el \
>   sh4 sh4eb s390x aarch64 aarch64_be hppa riscv32 riscv64 xtensa xtensaeb \
>   microblaze microblazeel or1k x86_64"
>
> +# check if given target CPUS is/are in the supported target list
> +qemu_check_target_list() {
> +    all="$qemu_target_list"
> +    if [ "$1" = "ALL" ]; then

Risky, if $1 can ever be "!" (I didn't trace whether the parameters for 
this function can come from a user deliberately trying to provoke a 
syntax error).  Better would be:

if [ "x$1" = xALL ]; then

> +      checked_target_list="$all"
> +      return
> +    fi
> +    list=""
> +    for target in $@; do
> +        unknown_target="true"
> +        for cpu in $all ; do
> +            if [ "$cpu" = "$target" ] ; then

Here you may be safer, since $cpu comes from $qemu_target_list which is 
under your control, but being consistent about always prefixing tests 
with leading x to avoid future changes from confusing test with 
something that might resemble an operator can't hurt.


> @@ -286,12 +315,22 @@ EOF
>   }
>
>   qemu_set_binfmts() {
> +    if [ "$1" = "NONE" ]; then

More instances. I'll quit pointing them out.

> +      return
> +    fi
> +
>       # probe cpu type
>       host_family=$(qemu_get_family)
>
> -    # register the interpreter for each cpu except for the native one
> +    # reduce the list of target interpreters to those given in the CLI
> +    targets="$@"
> +    if [ $# -eq 0 ]; then
> +      targets="ALL"
> +    fi
> +    qemu_check_target_list $(echo "$targets" | tr ',' ' ')

If desired, you could have used the shell's own splitting via IFS=, (and 
restoring IFS afterwards) instead of fork()ing out to echo|tr.


> +qemu_remove_interpreter() {
> +    if [ "$1" = "ALL" ]; then
> +      find /proc/sys/fs/binfmt_misc/ -type f -name 'qemu-*' -exec sh -c
> 'echo -1 > {}' \;

echo -1 is not portable (you are not guaranteed that echo won't try to 
treat it as an option); better would be using printf.

> +    else
> +        qemu_check_target_list $(echo "$1" | tr ',' ' ')
> +        for t in $checked_target_list; do
> +            find /proc/sys/fs/binfmt_misc/ -type f -name "qemu-$t" -exec
> sh -c 'echo -1 > {}' \;

and again

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org