Source-Changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/usr.bin/make



> From christos...
> 
> Well, I would leave it in. I would have preferred it if it was implemented
> as a getopt() replacement, enabled #ifdef BOOTSTRAP and placed in util.c
> 
> That way, the code does not change in the normal environment, and it
> uses the libc getopt() as intended.
> 
> christos

Sure, maintaining the exact getopt API would be nice, though there
is a counter-argument that, given a custom implementation, it might
be better to be upfront and clear about it. It's easier to read
and work on: what you see is what you get.  I can rework it to call
getopt_custom() or something, but that's not clean either, because
both main() and getopt_custom() would be making assumptions about
the others internal implementation. It seems better just to explicitly
parse the args, which took only a fraction of the lines it took to
talk about it.

I kept the getopt optstring, so it's functionally similar, it just
doesn't use quite the same binding.

If we do keep the general API, the name still has to be changed
because there is no getopt.h, it's in unistd.h and so there is no
good way to declare one that will always match local headers.

But the worst problem is the coverage issue, it's dangerous to
intentionally have two implementations and run one of them only in
certain obscure situations. I'm very surprised to hear you advocate
that, with a build kludge on top of it. We've accepted portability
as a goal, this is a consequence. 

This is a true case where a custom implementation is called for.
Unlike 99.?% of the arg-parsing programs, this one does special
non-posix stuff that isn't fully specified even in our getopt.
Rather than asking the library to work anyway, in this case I would
argue that the proper and standard-conforming approach is to
custom-code this one exceptional case.

Ross

>
>> Parse args with open code to eliminate use of getopt(3).
>> No functional change under NetBSD.
>> 
>> Restarting a getopt(3) loop is an extension to the posix getopt(3)
>> behavior and is not portable.
>> 
>> Fixes tools build (tools/groff) under Cygwin.




Home | Main Index | Thread Index | Old Index