getopt: refactor long-option handling

There were two copies of the bulk of the code to handle long options.
Now there is only one.  (Yes, this is in aid of merging from gnulib.)

The change to bug-getopt4.c clarifies the error messages when the test
fails.

	* posix/getopt.c (process_long_option): New function split out
	from _getopt_internal_r.
	(_getopt_internal_r): Replace both copies of the long-option
	processing code with calls to process_long_option.
	* posix/bug-getopt4.c (one_test): Print argv[0] in error messages.
	(do_test): Differentiate argv[0] in the two subtests.
This commit is contained in:
Zack Weinberg 2017-04-01 16:19:50 -04:00
parent c1af8775f2
commit dfbea09f96
3 changed files with 223 additions and 309 deletions

View File

@ -1,5 +1,12 @@
2017-04-07 Zack Weinberg <zackw@panix.com>
* posix/getopt.c (process_long_option): New function split out
from _getopt_internal_r.
(_getopt_internal_r): Replace both copies of the long-option
processing code with calls to process_long_option.
* posix/bug-getopt4.c (one_test): Print argv[0] in error messages.
(do_test): Differentiate argv[0] in the two subtests.
* posix/getopt_int.h (_getopt_data): Remove __posixly_correct field.
* posix/getopt.c (_getopt_internal_r): Move some initialization code...
(_getopt_initialize): ...here. Don't set d->__posixly_correct.

View File

@ -27,20 +27,20 @@ one_test (const char *fmt, int argc, char *argv[], int n, int expected[n])
int c = getopt_long (argc, argv, fmt, opts, NULL);
if (c != expected[i])
{
printf ("format '%s' test %d failed: expected '%c', got '%c'\n",
fmt, i, expected[i], c);
printf ("%s: format '%s' test %d failed: expected '%c', got '%c'\n",
argv[0], fmt, i, expected[i], c);
res = 1;
}
else if (optarg != NULL)
{
printf ("format '%s' test %d failed: optarg is \"%s\", not NULL\n",
fmt, i, optarg);
printf ("%s: format '%s' test %d failed: optarg is \"%s\", not NULL\n",
argv[0], fmt, i, optarg);
res = 1;
}
if (ftell (stderr) != 0)
{
printf ("format '%s' test %d failed: printed to stderr\n",
fmt, i);
printf ("%s: format '%s' test %d failed: printed to stderr\n",
argv[0], fmt, i);
res = 1;
}
}
@ -68,11 +68,11 @@ do_test (void)
remove (fname);
int ret = one_test ("W;", 2,
(char *[2]) { (char *) "bug-getopt4", (char *) "--a" },
(char *[2]) { (char *) "bug-getopt4a", (char *) "--a" },
1, (int [1]) { 'a' });
ret |= one_test ("W;", 3,
(char *[3]) { (char *) "bug-getopt4", (char *) "-W",
(char *[3]) { (char *) "bug-getopt4b", (char *) "-W",
(char *) "a" },
1, (int [1]) { 'a' });

View File

@ -179,7 +179,171 @@ exchange (char **argv, struct _getopt_data *d)
d->__last_nonopt = d->optind;
}
/* Initialize the internal data when the first call is made. */
/* Process the argument starting with d->__nextchar as a long option.
d->optind should *not* have been advanced over this argument.
If the value returned is -1, it was not actually a long option, the
state is unchanged, and the argument should be processed as a set
of short options (this can only happen when long_only is true).
Otherwise, the option (and its argument, if any) have been consumed
and the return value is the value to return from _getopt_internal_r. */
static int
process_long_option (int argc, char **argv, const char *optstring,
const struct option *longopts, int *longind,
int long_only, struct _getopt_data *d,
int print_errors, const char *prefix)
{
char *nameend;
size_t namelen;
const struct option *p;
const struct option *pfound = NULL;
struct option_list
{
const struct option *p;
struct option_list *next;
} *ambig_list = NULL;
int exact = 0;
int indfound = -1;
int option_index;
for (nameend = d->__nextchar; *nameend && *nameend != '='; nameend++)
/* Do nothing. */ ;
namelen = nameend - d->__nextchar;
/* Test all long options for either exact match
or abbreviated matches. */
for (p = longopts, option_index = 0; p->name; p++, option_index++)
if (!strncmp (p->name, d->__nextchar, namelen))
{
if (namelen == strlen (p->name))
{
/* Exact match found. */
pfound = p;
indfound = option_index;
exact = 1;
break;
}
else if (pfound == NULL)
{
/* First nonexact match found. */
pfound = p;
indfound = option_index;
}
else if (long_only
|| pfound->has_arg != p->has_arg
|| pfound->flag != p->flag
|| pfound->val != p->val)
{
/* Second or later nonexact match found. */
struct option_list *newp = alloca (sizeof (*newp));
newp->p = p;
newp->next = ambig_list;
ambig_list = newp;
}
}
if (ambig_list != NULL && !exact)
{
if (print_errors)
{
struct option_list first;
first.p = pfound;
first.next = ambig_list;
ambig_list = &first;
flockfile (stderr);
fprintf (stderr, _("%s: option '%s' is ambiguous; possibilities:"),
argv[0], argv[d->optind]);
do
{
fprintf (stderr, " '--%s'", ambig_list->p->name);
ambig_list = ambig_list->next;
}
while (ambig_list != NULL);
/* This must use 'fprintf' even though it's only printing a
single character, so that it goes through __fxprintf_nocancel
when compiled as part of glibc. */
fprintf (stderr, "\n");
funlockfile (stderr);
}
d->__nextchar += strlen (d->__nextchar);
d->optind++;
d->optopt = 0;
return '?';
}
if (pfound != NULL)
{
option_index = indfound;
d->optind++;
if (*nameend)
{
/* Don't test has_arg with >, because some C compilers don't
allow it to be used on enums. */
if (pfound->has_arg)
d->optarg = nameend + 1;
else
{
if (print_errors)
fprintf (stderr,
_("%s: option '%s%s' doesn't allow an argument\n"),
argv[0], prefix, pfound->name);
d->__nextchar += strlen (d->__nextchar);
d->optopt = pfound->val;
return '?';
}
}
else if (pfound->has_arg == 1)
{
if (d->optind < argc)
d->optarg = argv[d->optind++];
else
{
if (print_errors)
fprintf (stderr,
_("%s: option '%s%s' requires an argument\n"),
argv[0], prefix, pfound->name);
d->__nextchar += strlen (d->__nextchar);
d->optopt = pfound->val;
return optstring[0] == ':' ? ':' : '?';
}
}
d->__nextchar += strlen (d->__nextchar);
if (longind != NULL)
*longind = option_index;
if (pfound->flag)
{
*(pfound->flag) = pfound->val;
return 0;
}
return pfound->val;
}
/* Can't find it as a long option. If this is not getopt_long_only,
or the option starts with '--' or is not a valid short option,
then it's an error. */
if (!long_only || argv[d->optind][1] == '-'
|| strchr (optstring, *d->__nextchar) == NULL)
{
if (print_errors)
fprintf (stderr, _("%s: unrecognized option '%s%s'\n"),
argv[0], prefix, d->__nextchar);
d->__nextchar = NULL;
d->optind++;
d->optopt = 0;
return '?';
}
/* Otherwise interpret it as a short option. */
return -1;
}
/* Initialize internal data upon the first call to getopt. */
static const char *
_getopt_initialize (int argc, char **argv, const char *optstring,
@ -366,197 +530,46 @@ _getopt_internal_r (int argc, char **argv, const char *optstring,
}
/* We have found another option-ARGV-element.
Skip the initial punctuation. */
d->__nextchar = (argv[d->optind] + 1
+ (longopts != NULL && argv[d->optind][1] == '-'));
}
/* Decode the current option-ARGV-element. */
/* Check whether the ARGV-element is a long option.
If long_only and the ARGV-element has the form "-f", where f is
a valid short option, don't consider it an abbreviated form of
a long option that starts with f. Otherwise there would be no
way to give the -f short option.
On the other hand, if there's a long option "fubar" and
the ARGV-element is "-fu", do consider that an abbreviation of
the long option, just like "--fu", and not "-f" with arg "u".
This distinction seems to be the most useful approach. */
if (longopts != NULL
&& (argv[d->optind][1] == '-'
|| (long_only && (argv[d->optind][2]
|| !strchr (optstring, argv[d->optind][1])))))
{
char *nameend;
unsigned int namelen;
const struct option *p;
const struct option *pfound = NULL;
struct option_list
{
const struct option *p;
struct option_list *next;
} *ambig_list = NULL;
int exact = 0;
int indfound = -1;
int option_index;
for (nameend = d->__nextchar; *nameend && *nameend != '='; nameend++)
/* Do nothing. */ ;
namelen = nameend - d->__nextchar;
/* Test all long options for either exact match
or abbreviated matches. */
for (p = longopts, option_index = 0; p->name; p++, option_index++)
if (!strncmp (p->name, d->__nextchar, namelen))
{
if (namelen == (unsigned int) strlen (p->name))
{
/* Exact match found. */
pfound = p;
indfound = option_index;
exact = 1;
break;
}
else if (pfound == NULL)
{
/* First nonexact match found. */
pfound = p;
indfound = option_index;
}
else if (long_only
|| pfound->has_arg != p->has_arg
|| pfound->flag != p->flag
|| pfound->val != p->val)
{
/* Second or later nonexact match found. */
struct option_list *newp = alloca (sizeof (*newp));
newp->p = p;
newp->next = ambig_list;
ambig_list = newp;
}
}
if (ambig_list != NULL && !exact)
Check whether it might be a long option. */
if (longopts)
{
if (print_errors)
if (argv[d->optind][1] == '-')
{
struct option_list first;
first.p = pfound;
first.next = ambig_list;
ambig_list = &first;
flockfile (stderr);
fprintf (stderr,
_("%s: option '%s' is ambiguous; possibilities:"),
argv[0], argv[d->optind]);
do
{
fprintf (stderr, " '--%s'", ambig_list->p->name);
ambig_list = ambig_list->next;
}
while (ambig_list != NULL);
/* This must use 'fprintf' even though it's only printing a
single character, so that it goes through __fxprintf_nocancel
when compiled as part of glibc. */
fprintf (stderr, "\n");
funlockfile (stderr);
/* "--foo" is always a long option. The special option
"--" was handled above. */
d->__nextchar = argv[d->optind] + 2;
return process_long_option (argc, argv, optstring, longopts,
longind, long_only, d,
print_errors, "--");
}
/* If long_only and the ARGV-element has the form "-f",
where f is a valid short option, don't consider it an
abbreviated form of a long option that starts with f.
Otherwise there would be no way to give the -f short
option.
On the other hand, if there's a long option "fubar" and
the ARGV-element is "-fu", do consider that an
abbreviation of the long option, just like "--fu", and
not "-f" with arg "u".
This distinction seems to be the most useful approach. */
if (long_only && (argv[d->optind][2]
|| !strchr (optstring, argv[d->optind][1])))
{
int code;
d->__nextchar = argv[d->optind] + 1;
code = process_long_option (argc, argv, optstring, longopts,
longind, long_only, d,
print_errors, "-");
if (code != -1)
return code;
}
d->__nextchar += strlen (d->__nextchar);
d->optind++;
d->optopt = 0;
return '?';
}
if (pfound != NULL)
{
option_index = indfound;
d->optind++;
if (*nameend)
{
/* Don't test has_arg with >, because some C compilers don't
allow it to be used on enums. */
if (pfound->has_arg)
d->optarg = nameend + 1;
else
{
if (print_errors)
{
if (argv[d->optind - 1][1] == '-')
/* --option */
fprintf (stderr, _("\
%s: option '--%s' doesn't allow an argument\n"),
argv[0], pfound->name);
else
/* +option or -option */
fprintf (stderr, _("\
%s: option '%c%s' doesn't allow an argument\n"),
argv[0], argv[d->optind - 1][0],
pfound->name);
}
d->__nextchar += strlen (d->__nextchar);
d->optopt = pfound->val;
return '?';
}
}
else if (pfound->has_arg == 1)
{
if (d->optind < argc)
d->optarg = argv[d->optind++];
else
{
if (print_errors)
fprintf (stderr,
_("%s: option '--%s' requires an argument\n"),
argv[0], pfound->name);
d->__nextchar += strlen (d->__nextchar);
d->optopt = pfound->val;
return optstring[0] == ':' ? ':' : '?';
}
}
d->__nextchar += strlen (d->__nextchar);
if (longind != NULL)
*longind = option_index;
if (pfound->flag)
{
*(pfound->flag) = pfound->val;
return 0;
}
return pfound->val;
}
/* Can't find it as a long option. If this is not getopt_long_only,
or the option starts with '--' or is not a valid short
option, then it's an error.
Otherwise interpret it as a short option. */
if (!long_only || argv[d->optind][1] == '-'
|| strchr (optstring, *d->__nextchar) == NULL)
{
if (print_errors)
{
if (argv[d->optind][1] == '-')
/* --option */
fprintf (stderr, _("%s: unrecognized option '--%s'\n"),
argv[0], d->__nextchar);
else
/* +option or -option */
fprintf (stderr, _("%s: unrecognized option '%c%s'\n"),
argv[0], argv[d->optind][0], d->__nextchar);
}
d->__nextchar = (char *) "";
d->optind++;
d->optopt = 0;
return '?';
}
/* It is not a long option. Skip the initial punctuation. */
d->__nextchar = argv[d->optind] + 1;
}
/* Look at and handle the next short option-character. */
@ -576,28 +589,13 @@ _getopt_internal_r (int argc, char **argv, const char *optstring,
d->optopt = c;
return '?';
}
/* Convenience. Treat POSIX -W foo same as long option --foo */
if (temp[0] == 'W' && temp[1] == ';')
if (temp[0] == 'W' && temp[1] == ';' && longopts != NULL)
{
char *nameend;
const struct option *p;
const struct option *pfound = NULL;
int exact = 0;
int ambig = 0;
int indfound = 0;
int option_index;
if (longopts == NULL)
goto no_longs;
/* This is an option that requires an argument. */
if (*d->__nextchar != '\0')
{
d->optarg = d->__nextchar;
/* If we end this ARGV-element by taking the rest as an arg,
we must advance to the next element now. */
d->optind++;
}
d->optarg = d->__nextchar;
else if (d->optind == argc)
{
if (print_errors)
@ -613,103 +611,12 @@ _getopt_internal_r (int argc, char **argv, const char *optstring,
return c;
}
else
/* We already incremented 'd->optind' once;
increment it again when taking next ARGV-elt as argument. */
d->optarg = argv[d->optind++];
d->optarg = argv[d->optind];
/* optarg is now the argument, see if it's in the
table of longopts. */
for (d->__nextchar = nameend = d->optarg; *nameend && *nameend != '=';
nameend++)
/* Do nothing. */ ;
/* Test all long options for either exact match
or abbreviated matches. */
for (p = longopts, option_index = 0; p->name; p++, option_index++)
if (!strncmp (p->name, d->__nextchar, nameend - d->__nextchar))
{
if ((unsigned int) (nameend - d->__nextchar) == strlen (p->name))
{
/* Exact match found. */
pfound = p;
indfound = option_index;
exact = 1;
break;
}
else if (pfound == NULL)
{
/* First nonexact match found. */
pfound = p;
indfound = option_index;
}
else if (long_only
|| pfound->has_arg != p->has_arg
|| pfound->flag != p->flag
|| pfound->val != p->val)
/* Second or later nonexact match found. */
ambig = 1;
}
if (ambig && !exact)
{
if (print_errors)
fprintf (stderr, _("%s: option '-W %s' is ambiguous\n"),
argv[0], d->optarg);
d->__nextchar += strlen (d->__nextchar);
return '?';
}
if (pfound != NULL)
{
option_index = indfound;
if (*nameend)
{
/* Don't test has_arg with >, because some C compilers don't
allow it to be used on enums. */
if (pfound->has_arg)
d->optarg = nameend + 1;
else
{
if (print_errors)
fprintf (stderr, _("\
%s: option '-W %s' doesn't allow an argument\n"),
argv[0], pfound->name);
d->__nextchar += strlen (d->__nextchar);
return '?';
}
}
else if (pfound->has_arg == 1)
{
if (d->optind < argc)
d->optarg = argv[d->optind++];
else
{
if (print_errors)
fprintf (stderr, _("\
%s: option '-W %s' requires an argument\n"),
argv[0], pfound->name);
d->__nextchar += strlen (d->__nextchar);
return optstring[0] == ':' ? ':' : '?';
}
}
else
d->optarg = NULL;
d->__nextchar += strlen (d->__nextchar);
if (longind != NULL)
*longind = option_index;
if (pfound->flag)
{
*(pfound->flag) = pfound->val;
return 0;
}
return pfound->val;
}
no_longs:
d->__nextchar = NULL;
return 'W'; /* Let the application handle it. */
d->__nextchar = d->optarg;
d->optarg = NULL;
return process_long_option (argc, argv, optstring, longopts, longind,
0 /* long_only */, d, print_errors, "-W ");
}
if (temp[1] == ':')
{