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

[Unai Martinez Corral]

Hi Eric! Thanks for your suggestions.

2019/3/5 17:57, Eric Blake:

> > +    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

Certainly, I just kept the style that was used before all along the
script. Nonetheless, as you point out, fixing it does not hurt, so I
now made all the tests safe by preprending 'x' to both sides. No
matter where variables come from.

> > +    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.

Done. Now IFS is used inside qemu_check_target_list.

> > +      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.

Incidentally, I tried with MSYS2 and fedora:29 docker container. On
both of them , 'echo -1' works, while 'printf -1' does not. Anyway, I
replaced it with 'printf %s -1'. Hope it is ok.

Unai