]> Savannah Git Hosting - gnulib.git/commitdiff
getopt: refactor long-option handling
authorZack Weinberg <zackw@panix.com>
Thu, 6 Apr 2017 18:14:14 +0000 (11:14 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Thu, 6 Apr 2017 22:42:04 +0000 (15:42 -0700)
There were two copies of the bulk of the code to handle long options.
Now there is only one.

This change temporarily removes the logic to avoid using alloca when
standalone; the next patch in the series will restore it.

* lib/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.

ChangeLog
lib/getopt.c

index 1bbe41ecd4beaa442e5640b667aa4149f787839c..a1ad1bd6d813f958e99c14df8c7e4460866b6382 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2017-04-06  Zack Weinberg  <zackw@panix.com>
 
+       getopt: refactor long-option handling
+
+       There were two copies of the bulk of the code to handle long options.
+       Now there is only one.
+
+       This change temporarily removes the logic to avoid using alloca when
+       standalone; the next patch in the series will restore it.
+
+       * lib/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.
+
+
        getopt: tidy up _getopt_initialize a bit
 
        _getopt_data.__posixly_correct is completely redundant to
index 835767528f7b625d7c03617261783e6e6e02e232..51963746d95dca4ae89b3af788a0e5775b971181 100644 (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 _GL_UNUSED,
@@ -367,234 +531,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;
-#ifdef _LIBC
-/* malloc() not used for _LIBC to simplify failure messages.  */
-# define free_option_list(l)
-#else
-# define free_option_list(l)                   \
-      while (l != NULL)                                \
-       {                                       \
-         struct option_list *pn = l->next;     \
-         free (l);                             \
-         l = pn;                               \
-       }
-#endif
-      int exact = 0;
-      int ambig = 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 (ambig)
-             ; /* Taking simpler path to handling ambiguities.  */
-           else if (long_only
-                    || pfound->has_arg != p->has_arg
-                    || pfound->flag != p->flag
-                    || pfound->val != p->val)
-             {
-               /* Second or later nonexact match found.  */
-#ifdef _LIBC
-               struct option_list *newp = alloca (sizeof (*newp));
-#else
-               struct option_list *newp = malloc (sizeof (*newp));
-               if (newp == NULL)
-                 {
-                   free_option_list (ambig_list);
-                   ambig_list = NULL;
-                   ambig = 1; /* Use simpler fallback message.  */
-                 }
-               else
-#endif
-                 {
-                   newp->p = p;
-                   newp->next = ambig_list;
-                   ambig_list = newp;
-                 }
-             }
-         }
-
-      if ((ambig || ambig_list) && !exact)
+        Check whether it might be a long option.  */
+      if (longopts)
        {
-         if (print_errors && ambig_list)
+         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, "--");
            }
-         else if (print_errors && ambig)
-           {
-             fprintf (stderr,
-                      _("%s: option '%s' is ambiguous\n"),
-                      argv[0], argv[d->optind]);
-           }
-         d->__nextchar += strlen (d->__nextchar);
-         d->optind++;
-         d->optopt = 0;
-         free_option_list (ambig_list);
-         return '?';
-       }
 
-      free_option_list (ambig_list);
+         /* 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.
 
-      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;
-       }
+            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".
 
-      /* 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)
+            This distinction seems to be the most useful approach.  */
+         if (long_only && (argv[d->optind][2]
+                           || !strchr (optstring, argv[d->optind][1])))
            {
-             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);
+             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 = (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.  */
@@ -614,28 +590,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)
@@ -651,103 +612,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] == ':')
       {