Andrew Dunstan <andrew@dunslane.net> writes:
> It's working, but I don't think it's right :-) In particular, I don't
> believe this, or rather I don't believe that its converse is false:
> /* If not HAVE_GETOPT, we are using src/port/getopt.c, which has
> optreset */
Yeah, that was a 90% solution. I had first tried to look at whether
LIBOBJS contains "getopt.o", but it turns out that some versions of
autoconf actively prevent you from referencing that variable at all
in configure.in :-(.
> The trouble is that while we've forced use of our getopt.c, we haven't
> inhibited configure from checking the system's getopt. So on my test
> system HAVE_GETOPT is defined. Possibly I was wrong about needing to
> have optreset set with use of our port.
That's what I thought before.
At least in src/port/getopt.c (haven't looked at getopt_long), it would
only take one more line of code to make it safe against optreset not
being used. All that we need is an invariant that "place" is not
pointing at potentially volatile storage when we return -1, and most of
the returns have that already. (We don't really care about whether it
can be reset without a previous set of calls having been run to
completion, because we don't need that.)
If the same is true in getopt_long, then what I think we should do is
actually *remove* optreset from our implementations, and instead specify
that you can only switch to a new argv array after -1 has been returned.
That would remove a whole lot of unecessarily tense interactions with
the system libraries.
This is just in the nature of a cleanup so it probably needn't be
backpatched, unless people would rather that code be the same across all
branches (which does have its advantages).
Comments?
regards, tom lane