stty: fix a longstanding FIXME (was able to die half-way setting term params)

1 files changed, 241 insertions, 171 deletions

diff --git a/coreutils/stty.c b/coreutils/stty.c
index d709bfc..a63c1d1 100644
--- a/coreutils/stty.c
+++ b/coreutils/stty.c
@@ -173,7 +173,7 @@ struct mode_info {
 
 #define MI_ENTRY(N,T,F,B,M) { N, T, F, M, B }
 
-static const struct  mode_info mode_info[] = {
+static const struct mode_info mode_info[] = {
 	MI_ENTRY("parenb",   control,     REV,               PARENB,     0 ),
 	MI_ENTRY("parodd",   control,     REV,               PARODD,     0 ),
 	MI_ENTRY("cs5",      control,     0,                 CS5,     CSIZE),
@@ -333,7 +333,7 @@ struct control_info {
 
 /* Control characters. */
 
-static const struct  control_info control_info[] = {
+static const struct control_info control_info[] = {
 	{"intr",     CINTR,   VINTR},
 	{"quit",     CQUIT,   VQUIT},
 	{"erase",    CERASE,  VERASE},
@@ -380,9 +380,9 @@ enum {
 #define EMT(t) ((enum mode_type)(t))
 
 static const char *  visible(unsigned int ch);
-static int           recover_mode(char *arg, struct termios *mode);
+static int           recover_mode(const char *arg, struct termios *mode);
 static int           screen_columns(void);
-static int           set_mode(const struct mode_info *info,
+static void          set_mode(const struct mode_info *info,
 					int reversed, struct termios *mode);
 static speed_t       string_to_baud(const char *arg);
 static tcflag_t*     mode_type_flag(enum mode_type type, struct termios *mode);
@@ -398,7 +398,7 @@ static void          set_speed(enum speed_setting type,
 					const char *arg, struct termios *mode);
 static void          set_window_size(int rows, int cols);
 
-static const char *device_name;
+static const char *device_name = bb_msg_standard_input;
 
 static ATTRIBUTE_NORETURN void perror_on_device(const char *fmt)
 {
@@ -445,80 +445,192 @@ static const struct suffix_mult stty_suffixes[] = {
 	{NULL, 0   }
 };
 
+static const struct mode_info *find_mode(const char *name)
+{
+	int i;
+	for (i = 0; i < NUM_mode_info; ++i)
+		if (STREQ(name, mode_info[i].name))
+			return &mode_info[i];
+	return 0;
+}
+
+static const struct control_info *find_control(const char *name)
+{
+	int i;
+	for (i = 0; i < NUM_control_info; ++i)
+		if (STREQ(name, control_info[i].name))
+			return &control_info[i];
+	return 0;
+}
+
+enum {
+	param_need_arg = 0x80,
+	param_line   = 1 | 0x80,
+	param_rows   = 2 | 0x80,
+	param_cols   = 3 | 0x80,
+	param_size   = 4,
+	param_ispeed = 5 | 0x80,
+	param_ospeed = 6 | 0x80,
+	param_speed  = 7,
+};
+
+static int find_param(const char *name)
+{
+#ifdef HAVE_C_LINE
+	if (STREQ(name, "line")) return param_line;
+#endif
+#ifdef TIOCGWINSZ
+	if (STREQ(name, "rows")) return param_rows;
+	if (STREQ(name, "cols")) return param_cols;
+	if (STREQ(name, "columns")) return param_cols;
+	if (STREQ(name, "size")) return param_size;
+#endif
+	if (STREQ(name, "ispeed")) return param_ispeed;
+	if (STREQ(name, "ospeed")) return param_ospeed;
+	if (STREQ(name, "speed")) return param_speed;
+	return 0;
+}
+
+
 int stty_main(int argc, char **argv)
 {
 	struct termios mode;
 	void (*output_func)(struct termios *);
-	int    optc;
-	int    require_set_attr;
-	int    speed_was_set;
-	int    verbose_output;
-	int    recoverable_output;
-	int    k;
-	int    noargs = 1;
-	char * file_name = NULL;
+	const char *file_name = NULL;
+	int require_set_attr;
+	int speed_was_set;
+	int verbose_output;
+	int recoverable_output;
+	int noargs;
+	int k;
 
 	output_func = display_changed;
+	noargs = 1;
+	speed_was_set = 0;
+	require_set_attr = 0;
 	verbose_output = 0;
 	recoverable_output = 0;
 
-	/* Don't print error messages for unrecognized options.  */
-	opterr = 0;
-
-	while ((optc = getopt(argc, argv, "agF:")) != -1) {
-		switch (optc) {
-		case 'a':
-			verbose_output = 1;
-			output_func = display_all;
-			break;
-
-		case 'g':
-			recoverable_output = 1;
-			output_func = display_recoverable;
-			break;
+	/* First pass: only parse/verify command line params */
+	k = 0;
+	while (argv[++k]) {
+		const struct mode_info *mp;
+		const struct control_info *cp;
+		const char *arg = argv[k];
+		const char *argnext = argv[k+1];
+		int param;
+
+		if (arg[0] == '-') {
+			int i;
+			mp = find_mode(arg+1);
+			if (mp) {
+				if (!(mp->flags & REV))
+					bb_error_msg_and_die("invalid argument '%s'", arg);
+				noargs = 0;
+				continue;
+			}
+			/* It is an option - parse it */
+			i = 0;
+			while (arg[++i]) {
+				switch (arg[i]) {
+				case 'a':
+					verbose_output = 1;
+					output_func = display_all;
+					break;
+				case 'g':
+					recoverable_output = 1;
+					output_func = display_recoverable;
+					break;
+				case 'F':
+					if (file_name)
+						bb_error_msg_and_die("only one device may be specified");
+					file_name = &arg[i+1]; /* "-Fdevice" ? */
+					if (!file_name[0]) { /* nope, "-F device" */
+						int p = k+1; /* argv[p] is argnext */
+						file_name = argnext;
+						if (!file_name)
+							bb_error_msg_and_die(bb_msg_requires_arg, "-F");
+						/* remove -F param from arg[vc] */
+						--argc;
+						while (argv[p+1]) { argv[p] = argv[p+1]; ++p; }
+					}
+					goto end_option;
+				default:
+					bb_error_msg_and_die("invalid argument '%s'", arg);
+				}
+			}
+end_option:
+			continue;
+		}
 
-		case 'F':
-			if (file_name)
-				bb_error_msg_and_die("only one device may be specified");
-			file_name = optarg;
-			break;
+		mp = find_mode(arg);
+		if (mp) {
+			noargs = 0;
+			continue;
+		}
 
-		default:                /* unrecognized option */
+		cp = find_control(arg);
+		if (cp) {
+			if (!argnext)
+				bb_error_msg_and_die(bb_msg_requires_arg, arg);
 			noargs = 0;
-			break;
+			++k;
+			continue;
 		}
 
-		if (noargs == 0)
-			break;
-	}
+		param = find_param(arg);
+		if (param & param_need_arg) {
+			if (!argnext)
+				bb_error_msg_and_die(bb_msg_requires_arg, arg);
+			++k;
+		}
 
-	if (optind < argc)
+		switch (param) {
+#ifdef HAVE_C_LINE
+		case param_line:
+			bb_xparse_number(argnext, stty_suffixes);
+			break;
+#endif
+#ifdef TIOCGWINSZ
+		case param_rows:
+			bb_xparse_number(argnext, stty_suffixes);
+			break;
+		case param_cols:
+			bb_xparse_number(argnext, stty_suffixes);
+			break;
+		case param_size:
+#endif
+		case param_ispeed:
+		case param_ospeed:
+		case param_speed:
+			break;
+		default:
+			if (recover_mode(arg, &mode) == 1) break;
+			if (string_to_baud(arg) != (speed_t) -1) break;
+			bb_error_msg_and_die("invalid argument '%s'", arg);
+		}
 		noargs = 0;
+	}
 
-	/* Specifying both -a and -g gets an error.  */
-	if (verbose_output & recoverable_output)
-		bb_error_msg_and_die ("verbose and stty-readable output styles are mutually exclusive");
-
-	/* Specifying any other arguments with -a or -g gets an error.  */
-	if (~noargs & (verbose_output | recoverable_output))
-		bb_error_msg_and_die ("modes may not be set when specifying an output style");
-
-	/* FIXME: it'd be better not to open the file until we've verified
-	   that all arguments are valid.  Otherwise, we could end up doing
-	   only some of the requested operations and then failing, probably
-	   leaving things in an undesirable state.  */
+	/* Specifying both -a and -g is an error */
+	if (verbose_output && recoverable_output)
+		bb_error_msg_and_die("verbose and stty-readable output styles are mutually exclusive");
+	/* Specifying -a or -g with non-options is an error */
+	if (!noargs && (verbose_output || recoverable_output))
+		bb_error_msg_and_die("modes may not be set when specifying an output style");
 
+	/* Now it is safe to start doing things */
 	if (file_name) {
-		int fdflags;
-
+		int fd, fdflags;
 		device_name = file_name;
-		fclose(stdin);
-		xopen(device_name, O_RDONLY | O_NONBLOCK);
+		fd = xopen(device_name, O_RDONLY | O_NONBLOCK);
+		if (fd != 0) {
+			dup2(fd, 0);
+			close(fd);
+		}
 		fdflags = fcntl(STDIN_FILENO, F_GETFL);
 		if (fdflags == -1 || fcntl(STDIN_FILENO, F_SETFL, fdflags & ~O_NONBLOCK) < 0)
 			perror_on_device("%s: couldn't reset non-blocking mode");
-	} else {
-		device_name = bb_msg_standard_input;
 	}
 
 	/* Initialize to all zeroes so there is no risk memcmp will report a
@@ -527,122 +639,92 @@ int stty_main(int argc, char **argv)
 	if (tcgetattr(STDIN_FILENO, &mode))
 		perror_on_device("%s");
 
-	if (verbose_output | recoverable_output | noargs) {
+	if (verbose_output || recoverable_output || noargs) {
 		max_col = screen_columns();
 		current_col = 0;
 		output_func(&mode);
 		return EXIT_SUCCESS;
 	}
 
-	speed_was_set = 0;
-	require_set_attr = 0;
+	/* Second pass: perform actions */
 	k = 0;
-	while (++k < argc) {
-		int match_found = 0;
-		int reversed = 0;
-		int i;
-
-		if (argv[k][0] == '-') {
-			char *find_dev_opt;
-
-			++argv[k];
-
-     /* Handle "-a", "-ag", "-aF/dev/foo", "-aF /dev/foo", etc.
-	Find 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.
-     */
-			find_dev_opt = strchr(argv[k], 'F'); /* find -*F* */
-			if(find_dev_opt) {
-				if(find_dev_opt[1]==0)  /* -*F   /dev/foo */
-					k++;            /* skip  /dev/foo */
-				continue;   /* else -*F/dev/foo - no skip */
+	while (argv[++k]) {
+		const struct mode_info *mp;
+		const struct control_info *cp;
+		const char *arg = argv[k];
+		const char *argnext = argv[k+1];
+		int param;
+
+		if (arg[0] == '-') {
+			mp = find_mode(arg+1);
+			if (mp) {
+				set_mode(mp, 1 /* reversed */, &mode);
 			}
-			if(argv[k][0]=='a' || argv[k][0]=='g')
-				continue;
-			/* Is not options - is reverse params */
-			reversed = 1;
+			/* It is an option - already parsed. Skip it */
+			continue;
 		}
-		for (i = 0; i < NUM_mode_info; ++i)
-			if (STREQ(argv[k], mode_info[i].name)) {
-				match_found = set_mode(&mode_info[i], reversed, &mode);
-				require_set_attr = 1;
-				break;
-			}
 
-		if (match_found == 0 && reversed)
-			bb_error_msg_and_die("invalid argument `%s'", --argv[k]);
-
-		if (match_found == 0)
-			for (i = 0; i < NUM_control_info; ++i)
-				if (STREQ(argv[k], control_info[i].name)) {
-					if (k == argc - 1)
-					    bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-					match_found = 1;
-					++k;
-					set_control_char(&control_info[i], argv[k], &mode);
-					require_set_attr = 1;
-					break;
-				}
+		mp = find_mode(arg);
+		if (mp) {
+			set_mode(mp, 0 /* non-reversed */, &mode);
+			continue;
+		}
 
-		if (match_found == 0) {
-			if (STREQ(argv[k], "ispeed")) {
-				if (k == argc - 1)
-				    bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-				++k;
-				set_speed(input_speed, argv[k], &mode);
-				speed_was_set = 1;
-				require_set_attr = 1;
-			} else if (STREQ(argv[k], "ospeed")) {
-				if (k == argc - 1)
-				    bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-				++k;
-				set_speed(output_speed, argv[k], &mode);
-				speed_was_set = 1;
-				require_set_attr = 1;
-			}
-#ifdef TIOCGWINSZ
-			else if (STREQ(argv[k], "rows")) {
-				if (k == argc - 1)
-				    bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-				++k;
-				set_window_size((int) bb_xparse_number(argv[k], stty_suffixes),
-								-1);
-			} else if (STREQ(argv[k], "cols") || STREQ(argv[k], "columns")) {
-				if (k == argc - 1)
-				    bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-				++k;
-				set_window_size(-1,
-						(int) bb_xparse_number(argv[k], stty_suffixes));
-			} else if (STREQ(argv[k], "size")) {
-				max_col = screen_columns();
-				current_col = 0;
-				display_window_size(0);
-			}
-#endif
+		cp = find_control(arg);
+		if (cp) {
+			++k;
+			set_control_char(cp, argnext, &mode);
+			continue;
+		}
+
+		param = find_param(arg);
+		if (param & param_need_arg) {
+			++k;
+		}
+
+		switch (param) {
 #ifdef HAVE_C_LINE
-			else if (STREQ(argv[k], "line")) {
-				if (k == argc - 1)
-					bb_error_msg_and_die(bb_msg_requires_arg, argv[k]);
-				++k;
-				mode.c_line = bb_xparse_number(argv[k], stty_suffixes);
-				require_set_attr = 1;
-			}
+		case param_line:
+			mode.c_line = bb_xparse_number(argnext, stty_suffixes);
+			require_set_attr = 1;
+			break;
+#endif
+#ifdef TIOCGWINSZ
+		case param_cols:
+			set_window_size(-1, (int) bb_xparse_number(argnext, stty_suffixes));
+			break;
+		case param_size:
+			max_col = screen_columns();
+			current_col = 0;
+			display_window_size(0);
+			break;
+		case param_rows:
+			set_window_size((int) bb_xparse_number(argnext, stty_suffixes), -1);
+			break;
 #endif
-			else if (STREQ(argv[k], "speed")) {
-				max_col = screen_columns();
-				display_speed(&mode, 0);
-			} else if (recover_mode(argv[k], &mode) == 1)
+		case param_ispeed:
+			set_speed(input_speed, argnext, &mode);
+			speed_was_set = 1;
+			require_set_attr = 1;
+			break;
+		case param_ospeed:
+			set_speed(output_speed, argnext, &mode);
+			speed_was_set = 1;
+			require_set_attr = 1;
+			break;
+		case param_speed:
+			max_col = screen_columns();
+			display_speed(&mode, 0);
+			break;
+		default:
+			if (recover_mode(arg, &mode) == 1)
 				require_set_attr = 1;
-			else if (string_to_baud(argv[k]) != (speed_t) - 1) {
-				set_speed(both_speeds, argv[k], &mode);
+			else /* true: if (string_to_baud(arg) != (speed_t) -1) */ {
+				set_speed(both_speeds, arg, &mode);
 				speed_was_set = 1;
 				require_set_attr = 1;
-			} else
-				bb_error_msg_and_die("invalid argument `%s'", argv[k]);
+			} /* else - impossible (caught in the first pass):
+				bb_error_msg_and_die("invalid argument '%s'", arg); */
 		}
 	}
 
@@ -665,13 +747,6 @@ int stty_main(int argc, char **argv)
 		if (tcgetattr(STDIN_FILENO, &new_mode))
 			perror_on_device("%s");
 
-		/* Normally, one shouldn't use memcmp to compare structures that
-		   may have `holes' containing uninitialized data, but we have been
-		   careful to initialize the storage of these two variables to all
-		   zeroes.  One might think it more efficient simply to compare the
-		   modified fields, but that would require enumerating those fields --
-		   and not all systems have the same fields in this structure.  */
-
 		if (memcmp(&mode, &new_mode, sizeof(mode)) != 0) {
 #ifdef CIBAUD
 			/* SunOS 4.1.3 (at least) has the problem that after this sequence,
@@ -695,14 +770,11 @@ int stty_main(int argc, char **argv)
 
 /* Return 0 if not applied because not reversible; otherwise return 1.  */
 
-static int
+static void
 set_mode(const struct mode_info *info, int reversed, struct termios *mode)
 {
 	tcflag_t *bitsp;
 
-	if (reversed && (info->flags & REV) == 0)
-		return 0;
-
 	bitsp = mode_type_flag(EMT(info->type), mode);
 
 	if (bitsp == NULL) {
@@ -861,8 +933,6 @@ set_mode(const struct mode_info *info, int reversed, struct termios *mode)
 		*bitsp = *bitsp & ~((unsigned long)info->mask) & ~info->bits;
 	else
 		*bitsp = (*bitsp & ~((unsigned long)info->mask)) | info->bits;
-
-	return 1;
 }
 
 static void
@@ -1179,7 +1249,7 @@ static void display_recoverable(struct termios *mode)
 	putchar('\n');
 }
 
-static int recover_mode(char *arg, struct termios *mode)
+static int recover_mode(const char *arg, struct termios *mode)
 {
 	int i, n;
 	unsigned int chr;
@@ -1269,5 +1339,5 @@ static const char *visible(unsigned int ch)
 	}
 
 	*bpout = '\0';
-	return (const char *) buf;
+	return buf;
 }