Re: [Qemu-devel] [PATCH] qemu-io: Reinitialize optind correctly before parsing inner command.

[Eric Blake]

On 12/30/18 12:09 PM, Richard W.M. Jones wrote:
> On FreeBSD 11.2:
> 
>   $ ./qemu-io -f raw -c "aio_write 0 512" "nbd:localhost:10809"
>   Parsing error: non-numeric argument, or extraneous/unrecognized suffix -- 
> aio_write
> 
> After main option parsing, we reinitialize optind so we can parse each
> command.  The error happens when parsing the aio_write command.  After
> the aio_write getopt loop, optind == 0 and argv[optind] points to the
> command name ("aio_write" in this case).  The code fails because it
> tries to parse argv[optind] (which it thinks is the first argument) as
> an integer.
> 
> In fact optind _starts_ the loop as 0, because we set it to 0.
> 
> The FreeBSD manual page says:
> 
>   In order to use getopt() to evaluate multiple sets of arguments, or to
>   evaluate a single set of arguments multiple times, the variable optreset
>   must be set to 1 before the second and each additional set of calls to
>   getopt(), and the variable optind must be reinitialized.
> 
> (From the rest of the man page it is clear that optind must be
> reinitialized to 1).
> 
> Unfortunately this conflicts with the glibc man page which says:
> 
>   A program that scans multiple argument vectors,  or  rescans  the  same
>   vector  more than once, and wants to make use of GNU extensions such as
>   '+' and '-' at  the  start  of  optstring,  or  changes  the  value  of
>   POSIXLY_CORRECT  between scans, must reinitialize getopt() by resetting
>   optind to 0, rather than the traditional value of 1.  (Resetting  to  0
>   forces  the  invocation  of  an  internal  initialization  routine that
>   rechecks POSIXLY_CORRECT and checks for GNU extensions in optstring.)

The glibc manual implies that setting optind = 1 is a weak reset
(sufficient if you are NOT using either leading '+' or '-', and if you
do NOT expect POSIXLY_CORRECT to have changed value since the last
time), and that optind = 0 is a hard reset needed only when the weak
reset is insufficient.

> 
> Reinitialize optind to either 0 or 1 depending on whether we're using
> glibc or not.
> 
> I didn't set optreset - it's not present in glibc and it doesn't seem
> to make any difference on FreeBSD.

BSD has optreset as its way of forcing hard reset (instead of optind=0),
but both platforms allow optind=1 for soft reset.

> 
> Signed-off-by: Richard W.M. Jones <address@hidden>
> ---
>  qemu-io-cmds.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Given that none of our uses of getopt() in qemu-io-cmds.c rely on
leading '+' or '-', and that we don't call setenv() to change
POSIXLY_CORRECT on the fly, I think the simpler patch is to just blindly
set optind = 1 when we want a soft reset, as a hard reset is overkill
for our needs.

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 2c39124036..ca4e258579 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -114,7 +114,11 @@ static int command(BlockBackend *blk, const cmdinfo_t 
> *ct, int argc,
>          }
>      }
>  
> +#ifdef __GNU_LIBRARY__
>      optind = 0;
> +#else
> +    optind = 1;
> +#endif

and thus we don't need the #ifdef __GNU_LIBRARY__ code, nor the syntax
check violation.

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

signature.asc
Description: OpenPGP digital signature