[git commit] ash, hush: fold shell_builtin_read() way-too-many params into a struct param

Denys Vlasenko vda.linux at googlemail.com
Sun Aug 5 16:11:15 UTC 2018


commit: https://git.busybox.net/busybox/commit/?id=19358cc31317dca4642417066c1445ce00438e18
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

function                                             old     new   delta
getoptscmd                                           587     584      -3
readcmd                                              240     224     -16
shell_builtin_read                                  1426    1399     -27
builtin_read                                         210     182     -28
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-74)             Total: -74 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 shell/ash.c          | 38 ++++++++++++++----------------------
 shell/hush.c         | 33 +++++++++++--------------------
 shell/shell_common.c | 55 +++++++++++++++++++++++++---------------------------
 shell/shell_common.h | 22 +++++++++++----------
 4 files changed, 64 insertions(+), 84 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index 4641dfd19..f74bef6b1 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -13762,38 +13762,35 @@ letcmd(int argc UNUSED_PARAM, char **argv)
 static int FAST_FUNC
 readcmd(int argc UNUSED_PARAM, char **argv UNUSED_PARAM)
 {
-	char *opt_n = NULL;
-	char *opt_p = NULL;
-	char *opt_t = NULL;
-	char *opt_u = NULL;
-	char *opt_d = NULL; /* optimized out if !BASH */
-	int read_flags = 0;
+	struct builtin_read_params params;
 	const char *r;
 	int i;
 
+	memset(&params, 0, sizeof(params));
+
 	while ((i = nextopt("p:u:rt:n:sd:")) != '\0') {
 		switch (i) {
 		case 'p':
-			opt_p = optionarg;
+			params.opt_p = optionarg;
 			break;
 		case 'n':
-			opt_n = optionarg;
+			params.opt_n = optionarg;
 			break;
 		case 's':
-			read_flags |= BUILTIN_READ_SILENT;
+			params.read_flags |= BUILTIN_READ_SILENT;
 			break;
 		case 't':
-			opt_t = optionarg;
+			params.opt_t = optionarg;
 			break;
 		case 'r':
-			read_flags |= BUILTIN_READ_RAW;
+			params.read_flags |= BUILTIN_READ_RAW;
 			break;
 		case 'u':
-			opt_u = optionarg;
+			params.opt_u = optionarg;
 			break;
 #if BASH_READ_D
 		case 'd':
-			opt_d = optionarg;
+			params.opt_d = optionarg;
 			break;
 #endif
 		default:
@@ -13801,21 +13798,16 @@ readcmd(int argc UNUSED_PARAM, char **argv UNUSED_PARAM)
 		}
 	}
 
+	params.argv = argptr;
+	params.setvar = setvar0;
+	params.ifs = bltinlookup("IFS"); /* can be NULL */
+
 	/* "read -s" needs to save/restore termios, can't allow ^C
 	 * to jump out of it.
 	 */
  again:
 	INT_OFF;
-	r = shell_builtin_read(setvar0,
-		argptr,
-		bltinlookup("IFS"), /* can be NULL */
-		read_flags,
-		opt_n,
-		opt_p,
-		opt_t,
-		opt_u,
-		opt_d
-	);
+	r = shell_builtin_read(&params);
 	INT_ON;
 
 	if ((uintptr_t)r == 1 && errno == EINTR) {
diff --git a/shell/hush.c b/shell/hush.c
index 4c8814a01..3c19bceaa 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -10500,40 +10500,29 @@ static int FAST_FUNC builtin_type(char **argv)
 static int FAST_FUNC builtin_read(char **argv)
 {
 	const char *r;
-	char *opt_n = NULL;
-	char *opt_p = NULL;
-	char *opt_t = NULL;
-	char *opt_u = NULL;
-	char *opt_d = NULL; /* optimized out if !BASH */
-	const char *ifs;
-	int read_flags;
+	struct builtin_read_params params;
+
+	memset(&params, 0, sizeof(params));
 
 	/* "!": do not abort on errors.
 	 * Option string must start with "sr" to match BUILTIN_READ_xxx
 	 */
-	read_flags = getopt32(argv,
+	params.read_flags = getopt32(argv,
 #if BASH_READ_D
-		"!srn:p:t:u:d:", &opt_n, &opt_p, &opt_t, &opt_u, &opt_d
+		"!srn:p:t:u:d:", &params.opt_n, &params.opt_p, &params.opt_t, &params.opt_u, &params.opt_d
 #else
-		"!srn:p:t:u:", &opt_n, &opt_p, &opt_t, &opt_u
+		"!srn:p:t:u:", &params.opt_n, &params.opt_p, &params.opt_t, &params.opt_u
 #endif
 	);
-	if (read_flags == (uint32_t)-1)
+	if ((uint32_t)params.read_flags == (uint32_t)-1)
 		return EXIT_FAILURE;
 	argv += optind;
-	ifs = get_local_var_value("IFS"); /* can be NULL */
+	params.argv = argv;
+	params.setvar = set_local_var_from_halves;
+	params.ifs = get_local_var_value("IFS"); /* can be NULL */
 
  again:
-	r = shell_builtin_read(set_local_var_from_halves,
-		argv,
-		ifs,
-		read_flags,
-		opt_n,
-		opt_p,
-		opt_t,
-		opt_u,
-		opt_d
-	);
+	r = shell_builtin_read(&params);
 
 	if ((uintptr_t)r == 1 && errno == EINTR) {
 		unsigned sig = check_and_run_traps();
diff --git a/shell/shell_common.c b/shell/shell_common.c
index 0a07296f3..f2bf5ab65 100644
--- a/shell/shell_common.c
+++ b/shell/shell_common.c
@@ -46,16 +46,7 @@ int FAST_FUNC is_well_formed_var_name(const char *s, char terminator)
 //Here we can simply store "VAR=" at buffer start and store read data directly
 //after "=", then pass buffer to setvar() to consume.
 const char* FAST_FUNC
-shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
-	char       **argv,
-	const char *ifs,
-	int        read_flags,
-	const char *opt_n,
-	const char *opt_p,
-	const char *opt_t,
-	const char *opt_u,
-	const char *opt_d
-)
+shell_builtin_read(struct builtin_read_params *params)
 {
 	struct pollfd pfd[1];
 #define fd (pfd[0].fd) /* -u FD */
@@ -70,9 +61,13 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	int bufpos; /* need to be able to hold -1 */
 	int startword;
 	smallint backslash;
+	char **argv;
+	const char *ifs;
+	int read_flags;
 
 	errno = err = 0;
 
+	argv = params->argv;
 	pp = argv;
 	while (*pp) {
 		if (!is_well_formed_var_name(*pp, '\0')) {
@@ -84,29 +79,29 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	}
 
 	nchars = 0; /* if != 0, -n is in effect */
-	if (opt_n) {
-		nchars = bb_strtou(opt_n, NULL, 10);
+	if (params->opt_n) {
+		nchars = bb_strtou(params->opt_n, NULL, 10);
 		if (nchars < 0 || errno)
 			return "invalid count";
 		/* note: "-n 0": off (bash 3.2 does this too) */
 	}
 
 	end_ms = 0;
-	if (opt_t && !ENABLE_FEATURE_SH_READ_FRAC) {
-		end_ms = bb_strtou(opt_t, NULL, 10);
+	if (params->opt_t && !ENABLE_FEATURE_SH_READ_FRAC) {
+		end_ms = bb_strtou(params->opt_t, NULL, 10);
 		if (errno)
 			return "invalid timeout";
 		if (end_ms > UINT_MAX / 2048) /* be safely away from overflow */
 			end_ms = UINT_MAX / 2048;
 		end_ms *= 1000;
 	}
-	if (opt_t && ENABLE_FEATURE_SH_READ_FRAC) {
+	if (params->opt_t && ENABLE_FEATURE_SH_READ_FRAC) {
 		/* bash 4.3 (maybe earlier) supports -t N.NNNNNN */
 		char *p;
 		/* Eat up to three fractional digits */
 		int frac_digits = 3 + 1;
 
-		end_ms = bb_strtou(opt_t, &p, 10);
+		end_ms = bb_strtou(params->opt_t, &p, 10);
 		if (end_ms > UINT_MAX / 2048) /* be safely away from overflow */
 			end_ms = UINT_MAX / 2048;
 
@@ -128,13 +123,13 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	}
 
 	fd = STDIN_FILENO;
-	if (opt_u) {
-		fd = bb_strtou(opt_u, NULL, 10);
+	if (params->opt_u) {
+		fd = bb_strtou(params->opt_u, NULL, 10);
 		if (fd < 0 || errno)
 			return "invalid file descriptor";
 	}
 
-	if (opt_t && end_ms == 0) {
+	if (params->opt_t && end_ms == 0) {
 		/* "If timeout is 0, read returns immediately, without trying
 		 * to read any data. The exit status is 0 if input is available
 		 * on the specified file descriptor, non-zero otherwise."
@@ -147,14 +142,16 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 		return (const char *)(uintptr_t)(r <= 0);
 	}
 
-	if (opt_p && isatty(fd)) {
-		fputs(opt_p, stderr);
+	if (params->opt_p && isatty(fd)) {
+		fputs(params->opt_p, stderr);
 		fflush_all();
 	}
 
+	ifs = params->ifs;
 	if (ifs == NULL)
 		ifs = defifs;
 
+	read_flags = params->read_flags;
 	if (nchars || (read_flags & BUILTIN_READ_SILENT)) {
 		tcgetattr(fd, &tty);
 		old_tty = tty;
@@ -181,11 +178,11 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	retval = (const char *)(uintptr_t)0;
 	startword = 1;
 	backslash = 0;
-	if (opt_t)
+	if (params->opt_t)
 		end_ms += (unsigned)monotonic_ms();
 	buffer = NULL;
 	bufpos = 0;
-	delim = opt_d ? *opt_d : '\n';
+	delim = params->opt_d ? params->opt_d[0] : '\n';
 	do {
 		char c;
 		int timeout;
@@ -194,7 +191,7 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 			buffer = xrealloc(buffer, bufpos + 0x101);
 
 		timeout = -1;
-		if (opt_t) {
+		if (params->opt_t) {
 			timeout = end_ms - (unsigned)monotonic_ms();
 			/* ^^^^^^^^^^^^^ all values are unsigned,
 			 * wrapping math is used here, good even if
@@ -246,7 +243,7 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 		 * without variable names (bash compat).
 		 * Thus, "read" and "read REPLY" are not the same.
 		 */
-		if (!opt_d && argv[0]) {
+		if (!params->opt_d && argv[0]) {
 /* http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_05 */
 			const char *is_ifs = strchr(ifs, c);
 			if (startword && is_ifs) {
@@ -261,7 +258,7 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 			if (argv[1] != NULL && is_ifs) {
 				buffer[bufpos] = '\0';
 				bufpos = 0;
-				setvar(*argv, buffer);
+				params->setvar(*argv, buffer);
 				argv++;
 				/* can we skip one non-space ifs char? (2: yes) */
 				startword = isspace(c) ? 2 : 1;
@@ -313,14 +310,14 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 		}
 
 		/* Use the remainder as a value for the next variable */
-		setvar(*argv, buffer);
+		params->setvar(*argv, buffer);
 		/* Set the rest to "" */
 		while (*++argv)
-			setvar(*argv, "");
+			params->setvar(*argv, "");
 	} else {
 		/* Note: no $IFS removal */
 		buffer[bufpos] = '\0';
-		setvar("REPLY", buffer);
+		params->setvar("REPLY", buffer);
 	}
 
  ret:
diff --git a/shell/shell_common.h b/shell/shell_common.h
index 875fd9ea7..a1323021d 100644
--- a/shell/shell_common.h
+++ b/shell/shell_common.h
@@ -30,6 +30,17 @@ int FAST_FUNC is_well_formed_var_name(const char *s, char terminator);
 
 /* Builtins */
 
+struct builtin_read_params {
+	int        read_flags;
+	void FAST_FUNC (*setvar)(const char *name, const char *val);
+	char       **argv;
+	const char *ifs;
+	const char *opt_n;
+	const char *opt_p;
+	const char *opt_t;
+	const char *opt_u;
+	const char *opt_d;
+};
 enum {
 	BUILTIN_READ_SILENT = 1 << 0,
 	BUILTIN_READ_RAW    = 1 << 1,
@@ -40,16 +51,7 @@ enum {
 //	shell_builtin_read(setvar,argv,ifs,read_flags)
 //#endif
 const char* FAST_FUNC
-shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
-	char       **argv,
-	const char *ifs,
-	int        read_flags,
-	const char *opt_n,
-	const char *opt_p,
-	const char *opt_t,
-	const char *opt_u,
-	const char *opt_d
-);
+shell_builtin_read(struct builtin_read_params *params);
 
 int FAST_FUNC
 shell_builtin_ulimit(char **argv);


More information about the busybox-cvs mailing list