stty option-parsing bug fixes

[Paul Eggert]

While looking into coreutils's use of getopt, I noticed some bugs
in stty (not too surprising, as hacky as it is).  I installed this
patch to fix the bugs I found, and clean up the code a bit.

2004-09-05  Paul Eggert  <address@hidden>

        * src/stty.c (valid_options): Remove.
        (main): Fix some bugs in handling invalid option-combinations
        like "stty -F".
        * tests/stty/basic-1: Check for the fixed bugs.

Index: src/stty.c
===================================================================
RCS file: /home/eggert/coreutils/cu/src/stty.c,v
retrieving revision 1.125
diff -p -u -r1.125 stty.c
--- src/stty.c  3 Aug 2004 20:30:45 -0000       1.125
+++ src/stty.c  5 Sep 2004 07:17:21 -0000
@@ -712,34 +712,14 @@ settings, CHAR is taken literally, or co
   exit (status);
 }
 
-/* Return true if the string contains only valid options.  */
-static bool
-valid_options (char *opt, const char *valid_opts,
-              const char *valid_arg_opts)
-{
-  char ch;
-
-  if (*opt++ != '-' || *opt == 0)
-    return false;
-
-  while ((ch = *opt))
-    {
-      opt++;
-      if (strchr (valid_opts, ch))
-       continue;
-      if (strchr (valid_arg_opts, ch))
-       return true;
-      return false;
-    }
-  return true;
-}
-
 int
 main (int argc, char **argv)
 {
   struct termios mode;
   enum output_type output_type;
   int optc;
+  int argi = 0;
+  int opti = 1;
   bool require_set_attr;
   bool speed_was_set;
   bool verbose_output;
@@ -749,7 +729,6 @@ main (int argc, char **argv)
   char *file_name = NULL;
   int fd;
   const char *device_name;
-  bool posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);
 
   initialize_main (&argc, &argv);
   program_name = argv[0];
@@ -769,9 +748,17 @@ main (int argc, char **argv)
   /* Don't print error messages for unrecognized options.  */
   opterr = 0;
 
-  while ((optc = getopt_long (argc, argv, "agF:", longopts, NULL)) != -1)
+  /* If any new options are ever added to stty, the short options MUST
+     NOT allow any ambiguity with the stty settings.  For example, the
+     stty setting "-gagFork" would not be feasible, since it will be
+     parsed as "-g -a -g -F ork".  If you change anything about how
+     stty parses options, be sure it still works with combinations of
+     short and long options, --, POSIXLY_CORRECT, etc.  */
+
+  while ((optc = getopt_long (argc - argi, argv + argi, "-agF:",
+                             longopts, NULL))
+        != -1)
     {
-      bool unrecognized_option = false;
       switch (optc)
        {
        case 'a':
@@ -791,87 +778,22 @@ main (int argc, char **argv)
          break;
 
        default:
-         unrecognized_option = true;
-         break;
-       }
-
-      if (unrecognized_option)
-       break;
-    }
-
-  /* Clear out the options that have been parsed.  This is really
-     gross, but it's needed because stty SETTINGS look like options to
-     getopt(), so we need to work around things in a really horrible
-     way.  If any new options are ever added to stty, the short option
-     MUST NOT be a letter which is the first letter of one of the
-     possible stty settings.  If you change anything about how stty
-     parses options, be sure it still works with combinations of
-     short and long options, --, POSIXLY_CORRECT, etc.  */
-  for (k = 1; k < argc; k++)
-    {
-      size_t len;
-      char *eq;
+         noargs = false;
 
-      if (argv[k] == NULL)
-       continue;
+         /* Skip the argument containing this unrecognized option;
+            the 2nd pass will analyze it.  */
+         argi += opti;
+
+         /* Restart getopt_long from the first unskipped argument.  */
+         opti = 1;
+         optind = 0;
 
-      /* Handle --, and reset noargs if there are arguments following it.  */
-      if (STREQ (argv[k], "--"))
-       {
-         argv[k] = NULL;
-         if (k < argc - 1)
-           noargs = false;
          break;
        }
 
-      /* Handle "--file device" */
-      len = strlen (argv[k]);
-      if (len >= 3 && strstr ("--file", argv[k]))
-       {
-         argv[k] = NULL;
-         argv[k + 1] = NULL;
-         continue;
-       }
-
-      /* Handle "--all" and "--save".  */
-      if (len >= 3
-         && (strstr ("--all", argv[k])
-             || strstr ("--save", argv[k])))
-       {
-         argv[k] = NULL;
-         continue;
-       }
-
-      /* Handle "--file=device".  */
-      eq = strchr (argv[k], '=');
-      if (eq && eq - argv[k] >= 3)
-       {
-         *eq = '\0';
-         if (strstr ("--file", argv[k]))
-           {
-             argv[k] = NULL;
-             continue;
-           }
-         /* Put the equals sign back for the error message.  */
-         *eq = '=';
-       }
-
-      /* Handle "-a", "-ag", "-aF/dev/foo", "-aF /dev/foo", etc.  */
-      if (valid_options (argv[k], "ag", "F"))
-       {
-         /* FIXME: this loses when the device name ends in `F'.
-            e.g. `stty -F/dev/BARF nl' would clobber the `nl' argument.  */
-         if (argv[k][strlen (argv[k]) - 1] == 'F')
-           argv[k + 1] = NULL;
-         argv[k] = NULL;
-        }
-      /* Everything else must be a normal, non-option argument.  */
-      else
-       {
-         noargs = false;
-         if (posixly_correct)
-           break;
-        }
+      /* Clear fully-parsed arguments, so they don't confuse the 2nd pass.  */
+      while (opti < optind)
+       argv[argi + opti++] = NULL;
     }
 
   /* Specifying both -a and -g gets an error.  */
Index: tests/stty/basic-1
===================================================================
RCS file: /home/eggert/coreutils/cu/tests/stty/basic-1,v
retrieving revision 1.15
diff -p -u -r1.15 basic-1
--- tests/stty/basic-1  14 May 2003 06:27:35 -0000      1.15
+++ tests/stty/basic-1  5 Sep 2004 06:58:51 -0000
@@ -30,7 +30,12 @@ stty --save > $saved_state || fail=1
 stty `cat $saved_state` || fail=1
 
 # This would segfault prior to sh-utils-2.0j.
-stty erase -
+stty erase - || fail=1
+
+# These would improperly ignore invalid options through coreutils 5.2.1.
+stty -F 2>/dev/null && fail=1
+stty -raw -F no/such/file 2>/dev/null && fail=1
+stty -raw -a 2>/dev/null && fail=1
 
 # Build a list of all boolean options stty accepts on this system.
 options=`stty -a|tail -n +2|tr ';' '\n'|sed '/ = /d;s/^ //;s/-//g'`