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.

3 files changed, 219 insertions, 305 deletions

diff --git a/ChangeLog b/ChangeLog
index 5e9c79d49e..cf663d1b5f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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.
diff --git a/posix/bug-getopt4.c b/posix/bug-getopt4.c
index 1daffd1d34..0956ca57fb 100644
--- a/posix/bug-getopt4.c
+++ b/posix/bug-getopt4.c
@@ -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' });
 
diff --git a/posix/getopt.c b/posix/getopt.c
index 609638c193..ecb9923a6e 100644
--- a/posix/getopt.c
+++ b/posix/getopt.c
@@ -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;
+	      /* "--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, "--");
+	    }
 
-	      flockfile (stderr);
+	  /* 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.
 
-	      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 '?';
-	}
+	     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".
 
-      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)
+	     This distinction seems to be the most useful approach.  */
+	  if (long_only && (argv[d->optind][2]
+			    || !strchr (optstring, argv[d->optind][1])))
 	    {
-	      *(pfound->flag) = pfound->val;
-	      return 0;
+	      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;
 	    }
-	  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++];
-
-	/* 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;
-	  }
+	  d->optarg = argv[d->optind];
 
-      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] == ':')
       {