On 2/19/19 2:24 AM, Bernhard Voelker wrote: > On 2/18/19 11:20 AM, Assaf Gordon wrote: >> [...] what do you think ? > > Thanks for driving this. > > To Eric's suggestion, I'd remove the RESET_OPTIND function argument, > because it's never used. > Re. OPTIND: what about resetting the values of all involved externals > to their previous value? > > + int saved_opterr = opterr; > + int saved_optind = optind; > + int saved_optopt = optopt; > + char *saved_optarg = optarg; > > ... > > + /* Restore previous values. */ > + opterr = saved_opterr; > + optind = saved_optind; > + optopt = saved_optopt; > + optarg = saved_optarg;
Why? Most callers call this _instead_ of getopt_long(), so they don't care what things are left at except that optind must point to the first non-option. Restoring optind (to 1) is wrong. And for the rare caller that DOES want to call getopt_long() themselves afterwards, they can safe/restore as needed prior to calling this helper. Restoring state made sense as long as we had a RESET_OPTIND argument, but now I'm thinking that it does not buy us any utility at all - just blindly set opterr without worrying about restoring it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
