ash: deal with some TODOs (mostly trivial)

function                                             old     new   delta
msg_illnum                                             -      19     +19
evalvar                                             1365    1364      -1
illnum                                                19       -     -19
subevalvar                                          1182    1158     -24
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 0/2 up/down: 19/-44)            Total: -25 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

2 files changed, 62 insertions, 64 deletions

diff --git a/libbb/process_escape_sequence.c b/libbb/process_escape_sequence.c
index 6de2cacdd..11059d1a5 100644
--- a/libbb/process_escape_sequence.c
+++ b/libbb/process_escape_sequence.c
@@ -45,6 +45,9 @@ char FAST_FUNC bb_process_escape_sequence(const char **ptr)
 	}
 #endif
 
+	/* bash requires leading 0 in octal escapes:
+	 * \02 works, \2 does not (prints \ and 2).
+	 * We treat \2 as a valid octal escape sequence. */
 	do {
 		d = (unsigned char)(*q) - '0';
 #ifdef WANT_HEX_ESCAPES
@@ -80,7 +83,10 @@ char FAST_FUNC bb_process_escape_sequence(const char **ptr)
 				break;
 			}
 		} while (*++p);
-		n = *(p + (sizeof(charmap)/2));
+		/* p points to found escape char or NUL,
+		 * advance it and find what it translates to */
+		p += sizeof(charmap) / 2;
+		n = *p;
 	}
 
 	*ptr = q;
diff --git a/shell/ash.c b/shell/ash.c
index 1690c469b..524910950 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -112,7 +112,7 @@ enum { NOPTS = ARRAY_SIZE(optletters_optnames) };
 
 static const char homestr[] ALIGN1 = "HOME";
 static const char snlfmt[] ALIGN1 = "%s\n";
-static const char illnum[] ALIGN1 = "Illegal number: %s";
+static const char msg_illnum[] ALIGN1 = "Illegal number: %s";
 
 /*
  * We enclose jmp_buf in a structure so that we can declare pointers to
@@ -142,17 +142,10 @@ struct globals_misc {
 
 	struct jmploc *exception_handler;
 
-// disabled by vda: cannot understand how it was supposed to work -
-// cannot fix bugs. That's why you have to explain your non-trivial designs!
-//	/* do we generate EXSIG events */
-//	int exsig; /* counter */
-	volatile int suppressint; /* counter */
-// TODO: rename
-// pendingsig -> pending_sig
-// intpending -> pending_int
-	volatile /*sig_atomic_t*/ smallint intpending; /* 1 = got SIGINT */
+	volatile int suppress_int; /* counter */
+	volatile /*sig_atomic_t*/ smallint pending_int; /* 1 = got SIGINT */
 	/* last pending signal */
-	volatile /*sig_atomic_t*/ smallint pendingsig;
+	volatile /*sig_atomic_t*/ smallint pending_sig;
 	smallint exception_type; /* kind of exception (0..5) */
 	/* exceptions */
 #define EXINT 0         /* SIGINT received */
@@ -220,10 +213,9 @@ extern struct globals_misc *const ash_ptr_to_globals_misc;
 #define arg0        (G_misc.arg0       )
 #define exception_handler (G_misc.exception_handler)
 #define exception_type    (G_misc.exception_type   )
-#define suppressint       (G_misc.suppressint      )
-#define intpending        (G_misc.intpending       )
-//#define exsig             (G_misc.exsig            )
-#define pendingsig        (G_misc.pendingsig       )
+#define suppress_int      (G_misc.suppress_int     )
+#define pending_int       (G_misc.pending_int      )
+#define pending_sig       (G_misc.pending_sig      )
 #define isloginsh   (G_misc.isloginsh  )
 #define nullstr     (G_misc.nullstr    )
 #define optlist     (G_misc.optlist    )
@@ -283,7 +275,7 @@ static int isdigit_str9(const char *str)
  * more fun than worrying about efficiency and portability. :-))
  */
 #define INT_OFF do { \
-	suppressint++; \
+	suppress_int++; \
 	xbarrier(); \
 } while (0)
 
@@ -324,11 +316,11 @@ raise_interrupt(void)
 {
 	int ex_type;
 
-	intpending = 0;
+	pending_int = 0;
 	/* Signal is not automatically unmasked after it is raised,
 	 * do it ourself - unmask all signals */
 	sigprocmask_allsigs(SIG_UNBLOCK);
-	/* pendingsig = 0; - now done in onsig() */
+	/* pending_sig = 0; - now done in onsig() */
 
 	ex_type = EXSIG;
 	if (gotsig[SIGINT - 1] && !trap[SIGINT]) {
@@ -353,7 +345,7 @@ static IF_ASH_OPTIMIZE_FOR_SIZE(inline) void
 int_on(void)
 {
 	xbarrier();
-	if (--suppressint == 0 && intpending) {
+	if (--suppress_int == 0 && pending_int) {
 		raise_interrupt();
 	}
 }
@@ -362,18 +354,18 @@ static IF_ASH_OPTIMIZE_FOR_SIZE(inline) void
 force_int_on(void)
 {
 	xbarrier();
-	suppressint = 0;
-	if (intpending)
+	suppress_int = 0;
+	if (pending_int)
 		raise_interrupt();
 }
 #define FORCE_INT_ON force_int_on()
 
-#define SAVE_INT(v) ((v) = suppressint)
+#define SAVE_INT(v) ((v) = suppress_int)
 
 #define RESTORE_INT(v) do { \
 	xbarrier(); \
-	suppressint = (v); \
-	if (suppressint == 0 && intpending) \
+	suppress_int = (v); \
+	if (suppress_int == 0 && pending_int) \
 		raise_interrupt(); \
 } while (0)
 
@@ -685,7 +677,7 @@ trace_printf(const char *fmt, ...)
 	if (DEBUG_PID)
 		fprintf(tracefile, "[%u] ", (int) getpid());
 	if (DEBUG_SIG)
-		fprintf(tracefile, "pending s:%d i:%d(supp:%d) ", pendingsig, intpending, suppressint);
+		fprintf(tracefile, "pending s:%d i:%d(supp:%d) ", pending_sig, pending_int, suppress_int);
 	va_start(va, fmt);
 	vfprintf(tracefile, fmt, va);
 	va_end(va);
@@ -701,7 +693,7 @@ trace_vprintf(const char *fmt, va_list va)
 	if (DEBUG_PID)
 		fprintf(tracefile, "[%u] ", (int) getpid());
 	if (DEBUG_SIG)
-		fprintf(tracefile, "pending s:%d i:%d(supp:%d) ", pendingsig, intpending, suppressint);
+		fprintf(tracefile, "pending s:%d i:%d(supp:%d) ", pending_sig, pending_int, suppress_int);
 	vfprintf(tracefile, fmt, va);
 }
 
@@ -1556,7 +1548,7 @@ static int
 number(const char *s)
 {
 	if (!is_number(s))
-		ash_msg_and_raise_error(illnum, s);
+		ash_msg_and_raise_error(msg_illnum, s);
 	return atoi(s);
 }
 
@@ -2351,8 +2343,6 @@ setprompt(int whichprompt)
 #define CD_PHYSICAL 1
 #define CD_PRINT 2
 
-static int docd(const char *, int);
-
 static int
 cdopt(void)
 {
@@ -2360,7 +2350,7 @@ cdopt(void)
 	int i, j;
 
 	j = 'L';
-	while ((i = nextopt("LP"))) {
+	while ((i = nextopt("LP")) != '\0') {
 		if (i != j) {
 			flags ^= CD_PHYSICAL;
 			j = i;
@@ -3315,14 +3305,14 @@ onsig(int signo)
 {
 	gotsig[signo - 1] = 1;
 
-	if (/* exsig || */ (signo == SIGINT && !trap[SIGINT])) {
-		if (!suppressint) {
-			pendingsig = 0;
+	if (signo == SIGINT && !trap[SIGINT]) {
+		if (!suppress_int) {
+			pending_sig = 0;
 			raise_interrupt(); /* does not return */
 		}
-		intpending = 1;
+		pending_int = 1;
 	} else {
-		pendingsig = signo;
+		pending_sig = signo;
 	}
 }
 
@@ -3545,7 +3535,6 @@ getjob(const char *name, int getctl)
 	}
 
 	if (is_number(p)) {
-// TODO: number() instead? It does error checking...
 		num = atoi(p);
 		if (num < njobs) {
 			jp = jobtab + num - 1;
@@ -3917,7 +3906,7 @@ static int
 blocking_wait_with_raise_on_sig(struct job *job)
 {
 	pid_t pid = dowait(DOWAIT_BLOCK, job);
-	if (pid <= 0 && pendingsig)
+	if (pid <= 0 && pending_sig)
 		raise_exception(EXSIG);
 	return pid;
 }
@@ -4025,7 +4014,7 @@ jobscmd(int argc UNUSED_PARAM, char **argv)
 	int mode, m;
 
 	mode = 0;
-	while ((m = nextopt("lp"))) {
+	while ((m = nextopt("lp")) != '\0') {
 		if (m == 'l')
 			mode |= SHOW_PIDS;
 		else
@@ -4079,9 +4068,7 @@ waitcmd(int argc UNUSED_PARAM, char **argv)
 	int retval;
 	struct job *jp;
 
-//	exsig++;
-//	xbarrier();
-	if (pendingsig)
+	if (pending_sig)
 		raise_exception(EXSIG);
 
 	nextopt(nullstr);
@@ -4317,7 +4304,7 @@ cmdputs(const char *s)
 		if (!str)
 			continue;
  dostr:
-		while ((c = *str++)) {
+		while ((c = *str++) != '\0') {
 			USTPUTC(c, nextc);
 		}
 	}
@@ -6126,15 +6113,15 @@ subevalvar(char *p, char *str, int strloc, int subtype,
 #if ENABLE_ASH_BASH_COMPAT
 	case VSSUBSTR:
 		loc = str = stackblock() + strloc;
-// TODO: number() instead? It does error checking...
-		pos = atoi(loc);
+		/* Read POS in ${var:POS:LEN} */
+		pos = atoi(loc); /* number(loc) errors out on "1:4" */
 		len = str - startp - 1;
 
 		/* *loc != '\0', guaranteed by parser */
 		if (quotes) {
 			char *ptr;
 
-			/* We must adjust the length by the number of escapes we find. */
+			/* Adjust the length by the number of escapes */
 			for (ptr = startp; ptr < (str - 1); ptr++) {
 				if (*ptr == CTLESC) {
 					len--;
@@ -6145,15 +6132,22 @@ subevalvar(char *p, char *str, int strloc, int subtype,
 		orig_len = len;
 
 		if (*loc++ == ':') {
-// TODO: number() instead? It does error checking...
-			len = atoi(loc);
+			/* ${var::LEN} */
+			len = number(loc);
 		} else {
+			/* Skip POS in ${var:POS:LEN} */
 			len = orig_len;
-			while (*loc && *loc != ':')
+			while (*loc && *loc != ':') {
+				/* TODO?
+				 * bash complains on: var=qwe; echo ${var:1a:123}
+				if (!isdigit(*loc))
+					ash_msg_and_raise_error(msg_illnum, str);
+				 */
 				loc++;
-			if (*loc++ == ':')
-// TODO: number() instead? It does error checking...
-				len = atoi(loc);
+			}
+			if (*loc++ == ':') {
+				len = number(loc);
+			}
 		}
 		if (pos >= orig_len) {
 			pos = 0;
@@ -6372,7 +6366,7 @@ varvalue(char *name, int varflags, int flags, struct strlist *var_str_list)
 		ap = shellparam.p;
 		if (!ap)
 			return -1;
-		while ((p = *ap++)) {
+		while ((p = *ap++) != NULL) {
 			size_t partlen;
 
 			partlen = strlen(p);
@@ -6406,8 +6400,7 @@ varvalue(char *name, int varflags, int flags, struct strlist *var_str_list)
 	case '7':
 	case '8':
 	case '9':
-// TODO: number() instead? It does error checking...
-		num = atoi(name);
+		num = number(name);
 		if (num < 0 || num > shellparam.nparam)
 			return -1;
 		p = num ? shellparam.p[num - 1] : arg0;
@@ -6819,7 +6812,7 @@ expmeta(char *enddir, char *name)
 		p++;
 	if (*p == '.')
 		matchdot++;
-	while (!intpending && (dp = readdir(dirp)) != NULL) {
+	while (!pending_int && (dp = readdir(dirp)) != NULL) {
 		if (dp->d_name[0] == '.' && !matchdot)
 			continue;
 		if (pmatch(start, dp->d_name)) {
@@ -7213,8 +7206,8 @@ shellexec(char **argv, const char *path, int idx)
 		break;
 	}
 	exitstatus = exerrno;
-	TRACE(("shellexec failed for %s, errno %d, suppressint %d\n",
-		argv[0], e, suppressint));
+	TRACE(("shellexec failed for %s, errno %d, suppress_int %d\n",
+		argv[0], e, suppress_int));
 	ash_msg_and_raise(EXEXEC, "%s: %s", argv[0], errmsg(e, "not found"));
 	/* NOTREACHED */
 }
@@ -8016,7 +8009,7 @@ dotrap(void)
 	uint8_t savestatus;
 
 	savestatus = exitstatus;
-	pendingsig = 0;
+	pending_sig = 0;
 	xbarrier();
 
 	TRACE(("dotrap entered\n"));
@@ -8196,7 +8189,7 @@ evaltree(union node *n, int flags)
  out1:
 	if (checkexit & exitstatus)
 		evalskip |= SKIPEVAL;
-	else if (pendingsig && dotrap())
+	else if (pending_sig && dotrap())
 		goto exexit;
 
 	if (flags & EV_EXIT) {
@@ -9116,7 +9109,7 @@ evalcommand(union node *cmd, int flags)
 			if (i == EXINT)
 				exit_status = 128 + SIGINT;
 			if (i == EXSIG)
-				exit_status = 128 + pendingsig;
+				exit_status = 128 + pending_sig;
 			exitstatus = exit_status;
 			if (i == EXINT || spclbltin > 0) {
  raise:
@@ -9170,7 +9163,6 @@ evalbltin(const struct builtincmd *cmd, int argc, char **argv)
 	exitstatus |= ferror(stdout);
 	clearerr(stdout);
 	commandname = savecmdname;
-//	exsig = 0;
 	exception_handler = savehandler;
 
 	return i;
@@ -9221,7 +9213,7 @@ breakcmd(int argc UNUSED_PARAM, char **argv)
 	int n = argv[1] ? number(argv[1]) : 1;
 
 	if (n <= 0)
-		ash_msg_and_raise_error(illnum, argv[1]);
+		ash_msg_and_raise_error(msg_illnum, argv[1]);
 	if (n > loopnest)
 		n = loopnest;
 	if (n > 0) {
@@ -12712,7 +12704,7 @@ umaskcmd(int argc UNUSED_PARAM, char **argv)
 			mask = 0;
 			do {
 				if (*ap >= '8' || *ap < '0')
-					ash_msg_and_raise_error(illnum, argv[1]);
+					ash_msg_and_raise_error(msg_illnum, argv[1]);
 				mask = (mask << 3) + (*ap - '0');
 			} while (*++ap != '\0');
 			umask(mask);