stty: fix processing of options when -F is specified

This was a latent issue that became significant with
the addition of the -F option in FILEUTILS-3_16n-56-ge46a424

* src/stty.c (apply_settings): Refactor argument checking
to a function macro.  Augment the argument check to ignore
NULLed out arguments (already processed -F).
* NEWS: Mention the fix.
* tests/misc/stty-invalid.sh: Add a test case.
Fixes https://bugs.gnu.org/28859

diff --git a/NEWS b/NEWS
index 18922072abc894c50e42e500417e605079437a7d..a90b5691355f9b93959236a9bfde011e5c172a35 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   to attempt to hide the original length of the file name.
   [bug introduced in coreutils-8.28]
 
+  stty no longer crashes when processing settings with -F also specified.
+  [bug introduced in fileutils-4.0]
+
   timeout will again notice its managed command exiting, even when
   invoked with blocked CHLD signal, or in a narrow window where
   this CHLD signal from the exiting child was missed.  In each case

diff --git a/src/stty.c b/src/stty.c
index 48aac5967422cf6a4698c33a04afcc4db4f0ecc2..1a5c1e9620fe5e5e95004b5c81873388d7254cf2 100644 (file)
--- a/src/stty.c
+++ b/src/stty.c
@@ -1089,6 +1089,13 @@ apply_settings (bool checking, const char *device_name,
                 struct termios *mode, bool *speed_was_set,
                 bool *require_set_attr)
 {
+#define check_argument(arg) \
+  if (k == n_settings - 1 || ! settings[k+1]) \
+    { \
+      error (0, 0, _("missing argument to %s"), quote (arg)); \
+      usage (EXIT_FAILURE); \
+    }
+
   for (int k = 1; k < n_settings; k++)
     {
       char const *arg = settings[k];
@@ -1135,11 +1142,7 @@ apply_settings (bool checking, const char *device_name,
             {
               if (STREQ (arg, control_info[i].name))
                 {
-                  if (k == n_settings - 1)
-                    {
-                      error (0, 0, _("missing argument to %s"), quote (arg));
-                      usage (EXIT_FAILURE);
-                    }
+                  check_argument (arg);
                   match_found = true;
                   ++k;
                   set_control_char (&control_info[i], settings[k], mode);
@@ -1152,11 +1155,7 @@ apply_settings (bool checking, const char *device_name,
         {
           if (STREQ (arg, "ispeed"))
             {
-              if (k == n_settings - 1)
-                {
-                  error (0, 0, _("missing argument to %s"), quote (arg));
-                  usage (EXIT_FAILURE);
-                }
+              check_argument (arg);
               ++k;
               if (checking)
                 continue;
@@ -1166,11 +1165,7 @@ apply_settings (bool checking, const char *device_name,
             }
           else if (STREQ (arg, "ospeed"))
             {
-              if (k == n_settings - 1)
-                {
-                  error (0, 0, _("missing argument to %s"), quote (arg));
-                  usage (EXIT_FAILURE);
-                }
+              check_argument (arg);
               ++k;
               if (checking)
                 continue;
@@ -1198,11 +1193,7 @@ apply_settings (bool checking, const char *device_name,
 #ifdef TIOCGWINSZ
           else if (STREQ (arg, "rows"))
             {
-              if (k == n_settings - 1)
-                {
-                  error (0, 0, _("missing argument to %s"), quote (arg));
-                  usage (EXIT_FAILURE);
-                }
+              check_argument (arg);
               ++k;
               if (checking)
                 continue;
@@ -1212,11 +1203,7 @@ apply_settings (bool checking, const char *device_name,
           else if (STREQ (arg, "cols")
                    || STREQ (arg, "columns"))
             {
-              if (k == n_settings - 1)
-                {
-                  error (0, 0, _("missing argument to %s"), quote (arg));
-                  usage (EXIT_FAILURE);
-                }
+              check_argument (arg);
               ++k;
               if (checking)
                 continue;
@@ -1236,11 +1223,7 @@ apply_settings (bool checking, const char *device_name,
           else if (STREQ (arg, "line"))
             {
               unsigned long int value;
-              if (k == n_settings - 1)
-                {
-                  error (0, 0, _("missing argument to %s"), quote (arg));
-                  usage (EXIT_FAILURE);
-                }
+              check_argument (arg);
               ++k;
               mode->c_line = value = integer_arg (settings[k], ULONG_MAX);
               if (mode->c_line != value)

diff --git a/tests/misc/stty-invalid.sh b/tests/misc/stty-invalid.sh
index 06186e96a6529c3db64a2719dedf28e75b9ca4b7..5509b6523b13ff9ee99899f9c23f6f7f34e5d26c 100755 (executable)
--- a/tests/misc/stty-invalid.sh
+++ b/tests/misc/stty-invalid.sh
@@ -41,6 +41,15 @@ returns_ 1 stty $(echo $saved_state |sed 's/^[^:]*:/'$hex_2_64:/) \
 returns_ 1 stty $(echo $saved_state |sed 's/:[0-9a-f]*$/:'$hex_2_64/) \
   2>/dev/null || fail=1
 
+# From coreutils 5.3.0 to 8.28, the following would crash
+# due to incorrect argument handling.
+if tty -s </dev/tty; then
+  returns_ 1 stty eol -F /dev/tty || fail=1
+  returns_ 1 stty -F /dev/tty eol || fail=1
+  returns_ 1 stty -F/dev/tty eol || fail=1
+  returns_ 1 stty eol -F/dev/tty eol || fail=1
+fi
+
 # Just in case either of the above mistakenly succeeds (and changes
 # the state of our tty), try to restore the initial state.
 stty $saved_state || fail=1