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

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

[Richard W.M. Jones]

checkpatch says:

  WARNING: architecture specific defines should be avoided
  #60: FILE: qemu-io-cmds.c:117:
  +#ifdef __GNU_LIBRARY__

However I don't know a better way to fix this.

Rich.

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

[Richard W.M. Jones]

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

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.

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

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
     return ct->cfunc(blk, argc, argv);
 }
 
-- 
2.19.2

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

[Eric Blake]

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

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 <rjones@redhat.com>
> ---
>  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


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