Re: [PATCH] getopt: add support for non-option arguments

[Gianluca Anzolin <gianluca@sottospazio.it>]

Hello,

On Mon, Nov 24, 2014 at 09:48:01PM -0500, Rich Felker wrote:
> On Mon, Nov 24, 2014 at 07:39:05PM +0100, Gianluca Anzolin wrote:
> > diff --git a/src/misc/getopt_long.c b/src/misc/getopt_long.c
> > index 4ef5a5c..d8b2b66 100644
> > --- a/src/misc/getopt_long.c
> > +++ b/src/misc/getopt_long.c
> > @@ -12,7 +12,11 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
> >  		__optpos = 0;
> >  		optind = 1;
> >  	}
> > -	if (optind >= argc || !argv[optind] || argv[optind][0] != '-') return -1;
> > +	if (optind >= argc || !argv[optind]) return -1;
> > +
> > +	if (argv[optind][0] != '-')
> > +		return getopt(argc, argv, optstring);
> > +
> >  	if ((longonly && argv[optind][1]) ||
> >  		(argv[optind][1] == '-' && argv[optind][2]))
> >  	{
> > -- 
> > 2.1.3
> 
> For getopt_long, this introduces a second code path to return
> getopt(...). Wouldn't it be cleaner just to add an additional
> condition of (argv[optind][0] == '-') to the if statement containing
> the main body of the function?

Yes, that seems better, I will do it.

> I'm a little bit uncertain what's really best to do with getopt_long,
> but since there's a pending patch for abbreviated long options and
> there's demand to add GNU-style argv permutation to getopt_long (but
> not plain getopt), this code is likely to need major changes anyway
> in this release cycle, so I'm not too concerned that we do it the
> "best possible way" right now anyway. So if it works we can probably
> go with it.
> 

I tested the changes and verified that they work correctly. However I found
something worth mentioning, not caused by the patch itself.

For a command line like the following (optstring="a:b:c:def")

$ ./test -a 1 -b 2 -c 3 -d -e -f

for the options d, e and f the variable optarg is "3". Maybe this is the
intended behaviour, after all those options aren't supposed to get an argument,
but glibc always sets optarg to NULL in these cases.

> I'll see if anyone else has comments and if not commit (possibly with
> the above modification to getopt_long?) soon.
> 
> Rich

I'll send a new patch soon,

Thanks

Gianluca