Re: [Qemu-block] [PATCH v2] qemu-io: Reinitialize optind to 1 (not 0) before parsing inner command.

[Max Reitz]

On 07.01.19 19:45, Eric Blake wrote:
> On 1/7/19 12:14 PM, Max Reitz wrote:
>> On 07.01.19 18:59, Eric Blake wrote:
>>> On 1/7/19 11:50 AM, Max Reitz wrote:
>>>
>>>>>>> Note I didn't set optreset.  It's not present in glibc and the "hard
>>>>>>> reset" is not necessary in this context.
>>>>>>
>>>>>> But it sure sounds like FreeBSD requires you to set it, doesn't it?
>>>
>>> No.  Quoting https://www.freebsd.org/cgi/man.cgi?getopt(3)
>>>
>>>      The variables opterr and optind are both initialized to 1.      The 
>>> optind
>>>      variable may be set to another value before a set of calls     to
>>> getopt() in
>>>      order to skip over     more or less argv entries.
>>>
>>> so resetting it to 1 as a soft reset is no different to setting it to 2
>>> to skip argv[1].
>>
>> In theory it is very much different because the text clearly says "in
>> order to skip", not "in order to re-parse or use a different argv".
>> Especially the fact that we use different argvs is something that
>> implementations may not expect.
> 
> Consider the following input:
> 
> ./prog -ab -cd -ef
> 
> against
> 
> while ((opt = getopt(argc, argv, "abcdef")) != -1) {
>   switch (opt) {
>    case 'a': case 'b': case 'f': break;
>    case 'c': optind = 3; break;
>    case 'd': case 'e': abort();
>   }
> }
> 
> What does that do on BSD?  On glibc, after the third call, optind is
> still 2 (but I hard-set it to 3), then the fourth call returns 'd' and
> increments optind to 4, before abort()ing, never reaching 'e' or 'f'.
> But if BSD goes from 'c' to 'f' and skips 'd' and 'e', it is because BSD
> tracks internal state differently from glibc.  Either way, the fact that
> setting optind = 3 does NOT make glibc return 'e' or 'f' means that I
> did NOT skip ahead to argument 3 (glibc still returned 'd' then skipped
> to argument 4; either BSD does the same, or BSD skips to 'f'), and thus
> I can argue that the BSD man page is incomplete, and SHOULD be corrected
> to mention that assigning to optind to skip to a future argument is safe
> ONLY when the hidden state is not affected by being mid-parse of merged
> short options.  But how do you get out of the hidden state of merged
> short options? By parsing until getopt() returns -1.  And once you've
> reached that point, then hidden state is clear, and skipping backwards
> is just as reasonable as skipping forwards.

Is it?

Why wouldn't FreeBSD just set optind to -1 at that point and just have
an "if (optind < 0) { return -1; }" at the beginning of getopt(),
without having cleared the internal state?

I agree that the variable _sp you define below should be reset, because
that is the sane thing to do whenever you jump to a new index in argv[]
(i.e. a new optind).  But that doesn't guarantee to me that there isn't
additional state that may not be cleared.

No, I cannot imagine why there should be such additional state.  I'm
just arguing based on the interface description, i.e. the man page.

>>> I think the BSD man page needs updating, and that will probably happen
>>> if I file my promised POSIX defect.
>>
>> Sure.  But as it is, it doesn't tell me that resetting optind to 1 is
>> sufficient to be able to parse a new argv.
> 
> But arguing that something that worked for Richard's testing is wrong,
> without reading the BSD source code, isn't going to help us either.

Come on, it's not like I'm demanding something impossible when I'm
asking for a weak global optreset. ;-)

>>> I don't see the point - Richard has already tested that optind = 1
>>> worked on BSD machines for our purposes, so we don't have to worry about
>>> the hard reset aspect of optreset=1.
>>
>> Well, and as far as I remember glibc's memcpy() at one point only copied
>> in one direction and things broke badly once they reversed it at some
>> point for some CPUs.
> 
> That was because of buggy software that didn't read the function
> contracts, and should have been using memmove( insta

Well, and right now this here is a patch that does not conform to the
function contract and that should use optreset = 1 in addition.

You're arguing with "Why would it not work?".  And I just feel like you
could have argued the same for memcpy(), "Why would it copy backwards?".

Please don't take this as an accusation, but I feel like you are trying
to make a point of how the standard should be.  I can fully understand
that, but I personally don't feel like qemu is the place to make such a
point (until the standard truly has been fixed).

Sure, this is a minor issue and I don't mind if someone else (i.e.
Kevin) merges it.  But maybe I want to make a point, too, which is to
take interface descriptions as they are, not as they should be.  Making
the interface more nice for all user software is distinct from making
our user software just assume it is nice already.

>> Just because it works now doesn't mean it will work always if the
>> specification allows for different behavior.
> 
> Yes, but that's why I need to file a POSIX defect, so that BSD won't
> change their current behavior because POSIX will require the soft reset
> behavior.
> 
> Here's the current thread on the POSIX list:
> https://www.mail-archive.com/address@hidden/msg03210.html
> 
> which I hope to turn into a formal defect soon.

Thanks!

Max

signature.asc
Description: OpenPGP digital signature