commit f49c728dc975cbf7cdeb84ee8402b01fec2e37d6
Author:     Jacob Welsh <jfw>
AuthorDate: Thu Oct 30 23:36:17 2025 +0000
Commit:     Jacob Welsh <jfw>
CommitDate: Fri Oct 31 01:27:24 2025 +0000

    busybox/coreutils: fix garbled stty output & nonsensical error messages, such as when querying a serial console device; prune ifdefs and straighten some error reporting.
    
    Example of the primary pathology:
    
            $ stty -a -F /dev/ttyUSB0
            speed 115200 baud;stty: /dev/ttyUSB0
            line = 0;
            intr = ^C; ...
    
            $ stty -a -F /dev/ttyUSB0 >/dev/null
            stty: /dev/ttyUSB0: Not a tty
    
            $ stty -F /dev/ttyUSB0 size
            stty: /dev/ttyUSB0
    
    Now shows:
    
            $ stty -a -F /dev/ttyUSB0
            speed 115200 baud; rows 0; columns 0; line = 0;
            intr = ^C; ...
    
    Cutting through the perror_on_device* wrappers to the better fitting busybox routines brings joint benefits of reducing format string warnings along with source and machine code size. The final one with "cannot perform all requested operations" is further improved by skipping perror because errno is not meaningful in that case.
    
    Text size -65 bytes on amd64.

diff --git a/base/busybox/coreutils/stty.c b/base/busybox/coreutils/stty.c
index 378a848..59318b0 100644
--- a/base/busybox/coreutils/stty.c
+++ b/base/busybox/coreutils/stty.c
@@ -796,16 +796,6 @@ static void set_speed_or_die(enum speed_setting type, const char *arg,
 	}
 }
 
-static NORETURN void perror_on_device_and_die(const char *fmt)
-{
-	bb_perror_msg_and_die(fmt, G.device_name);
-}
-
-static void perror_on_device(const char *fmt)
-{
-	bb_perror_msg(fmt, G.device_name);
-}
-
 /* Print format string MESSAGE and optional args.
    Wrap to next line first if it won't fit.
    Print a space first unless MESSAGE will start a new line */
@@ -844,7 +834,7 @@ static void newline(void)
 		wrapf("\n");
 }
 
-#ifdef TIOCGWINSZ
+/* Here and elsewhere there was an #ifdef TIOCGWINSZ, but that ioctl is already required elsewhere in busybox, and it wouldn't have worked to do without it here because display_window_size required it indirectly and wasn't thus guarded. Conditionals removed. */
 static void set_window_size(int rows, int cols)
 {
 	struct winsize win = { 0, 0, 0, 0 };
@@ -863,22 +853,21 @@ static void set_window_size(int rows, int cols)
 
 	if (ioctl(STDIN_FILENO, TIOCSWINSZ, (char *) &win))
 bail:
-		perror_on_device("%s");
+		bb_simple_perror_msg(G.device_name);
 }
-#endif
 
 static void display_window_size(int fancy)
 {
-	const char *fmt_str = "%s\0%s: no size information for this device";
-	unsigned width, height;
+	struct winsize win;
 
-	if (get_terminal_width_height(STDIN_FILENO, &width, &height)) {
-		if ((errno != EINVAL) || ((fmt_str += 2), !fancy)) {
-			perror_on_device(fmt_str);
-		}
+	/* This previously used get_terminal_width_height, but that's inappropriate for a number of reasons, resulting in nonsensical output e.g. when used with a serial port where rows and columns may legitimately be uninitialized (zero). */
+	if (ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == -1) {
+		/* Error handling here was contorted, appeared to have an off-by-one on its format string pointer hacking, and still didn't make much sense even after untangling. Simplified. */
+		if (!fancy)
+			bb_simple_perror_msg(G.device_name);
 	} else {
 		wrapf(fancy ? "rows %u; columns %u;" : "%u %u\n",
-				height, width);
+				(unsigned) win.ws_row, (unsigned) win.ws_col);
 	}
 }
 
@@ -1344,19 +1333,13 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 		switch (param) {
 #ifdef __linux__
 		case param_line:
-# ifndef TIOCGWINSZ
-			xatoul_range_sfx(argnext, 1, INT_MAX, stty_suffixes);
-			break;
-# endif /* else fall-through */
 #endif
-#ifdef TIOCGWINSZ
 		case param_rows:
 		case param_cols:
 		case param_columns:
 			xatoul_range_sfx(argnext, 1, INT_MAX, stty_suffixes);
 			break;
 		case param_size:
-#endif
 		case param_speed:
 			break;
 		case param_ispeed:
@@ -1400,7 +1383,7 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 	   spurious difference in an uninitialized portion of the structure */
 	memset(&mode, 0, sizeof(mode));
 	if (tcgetattr(STDIN_FILENO, &mode))
-		perror_on_device_and_die("%s");
+		bb_simple_perror_msg_and_die(G.device_name);
 
 	if (stty_state & (STTY_verbose_output | STTY_recoverable_output | STTY_noargs)) {
 		get_terminal_width_height(STDOUT_FILENO, &G.max_col, NULL);
@@ -1454,7 +1437,6 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 			stty_state |= STTY_require_set_attr;
 			break;
 #endif
-#ifdef TIOCGWINSZ
 		case param_cols:
 		case param_columns:
 			set_window_size(-1, xatoul_sfx(argnext, stty_suffixes));
@@ -1465,7 +1447,6 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 		case param_rows:
 			set_window_size(xatoul_sfx(argnext, stty_suffixes), -1);
 			break;
-#endif
 		case param_speed:
 			display_speed(&mode, 0);
 			break;
@@ -1492,7 +1473,7 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 		struct termios new_mode;
 
 		if (tcsetattr(STDIN_FILENO, TCSADRAIN, &mode))
-			perror_on_device_and_die("%s");
+			bb_simple_perror_msg_and_die(G.device_name);
 
 		/* POSIX (according to Zlotnick's book) tcsetattr returns zero if
 		   it performs *any* of the requested operations.  This means it
@@ -1505,7 +1486,7 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 		   spurious difference in an uninitialized portion of the structure */
 		memset(&new_mode, 0, sizeof(new_mode));
 		if (tcgetattr(STDIN_FILENO, &new_mode))
-			perror_on_device_and_die("%s");
+			bb_simple_perror_msg_and_die(G.device_name);
 
 		if (memcmp(&mode, &new_mode, sizeof(mode)) != 0) {
 /*
@@ -1527,7 +1508,7 @@ int stty_main(int argc UNUSED_PARAM, char **argv)
 			if ((stty_state & STTY_speed_was_set)
 			 || memcmp(&mode, &new_mode, sizeof(mode)) != 0)
 #endif
-				perror_on_device_and_die("%s: cannot perform all requested operations");
+				bb_error_msg_and_die("%s: cannot perform all requested operations", G.device_name);
 		}
 	}