Re: [Qemu-devel] [PATCH v4] qemu-io: Add generic function for reinitializing optind.

[Max Reitz]

On 18.01.19 11:11, Richard W.M. Jones wrote:
> On FreeBSD 11.2:
> 
>   $ nbdkit memory size=1M --run './qemu-io -f raw -c "aio_write 0 512" $nbd'
>   Parsing error: non-numeric argument, or extraneous/unrecognized suffix -- 
> aio_write
> 
> After main option parsing, we reinitialize optind so we can parse each
> command.  However reinitializing optind to 0 does not work on FreeBSD.
> What happens when you do this is optind remains 0 after the option
> parsing loop, and the result is we try to parse argv[optind] ==
> argv[0] == "aio_write" as if it was the first parameter.
> 
> 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).
> 
> The glibc man page 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.)
> 
> This commit introduces an OS-portability function called
> qemu_reset_optind which provides a way of resetting optind that works
> on FreeBSD and platforms that use optreset, while keeping it the same
> as now on other platforms.
> 
> Note that the qemu codebase sets optind in many other places, but in
> those other places it's setting a local variable and not using getopt.
> This change is only needed in places where we are using getopt and the
> associated global variable optind.
> 
> Signed-off-by: Richard W.M. Jones <address@hidden>
> ---
>  configure            | 14 ++++++++++++++
>  include/qemu/osdep.h | 16 ++++++++++++++++
>  qemu-img.c           |  2 +-
>  qemu-io-cmds.c       |  2 +-
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 3eee3fcf70..3d46df1517 100755

(I'm not quite sure where along the way just using a weak optreset was
discarded, but, oh well, this does make the code less ugly.)

[...]

> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 80df7253db..840af09cb0 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -109,6 +109,7 @@ extern int daemon(int, int);
>  #include <ctype.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <getopt.h>
>  #include <sys/stat.h>
>  #include <sys/time.h>
>  #include <assert.h>
> @@ -604,4 +605,19 @@ extern int qemu_icache_linesize_log;
>  extern int qemu_dcache_linesize;
>  extern int qemu_dcache_linesize_log;
>  
> +/*
> + * After using getopt or getopt_long, if you need to parse another set
> + * of options, then you must reset optind.  Unfortunately the way to
> + * do this varies between implementations of getopt.
> + */
> +static inline void qemu_reset_optind(void)
> +{
> +#ifdef HAVE_OPTRESET
> +    optind = 1;
> +    optreset = 1;
> +#else
> +    optind = 0;

So I take it this is supposed to always do a hard reset -- because if it
isn't, it might have been better to just always set optind = 1 as Eric
suggested.  But googling suggests OpenBSD and NetBSD both have optreset
as well, and newlib accepts optind = 0 (and apparently did not accept
optind = 1 in the past?), so I think this is good for that purpose.


So thanks a lot (and sorry about me being so stupid about
everything...), I've applied the patch to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max

> +#endif
> +}
> +
>  #endif

signature.asc
Description: OpenPGP digital signature