hush: deal with some easier TODOs

function                                             old     new   delta
is_well_formed_var_name                                -      87     +87
builtin_read                                          49      86     +37
die_if_script                                          -      31     +31
syntax_error_unterminated                              -      28     +28
syntax_error                                          26      51     +25
done_word                                            768     788     +20
syntax_error_at                                        -      12     +12
parse_stream_dquoted                                 320     328      +8
expand_variables                                    2064    2063      -1
run_list                                            1225    1220      -5
add_till_closing_paren                               308     303      -5
add_till_backquote                                   111     106      -5
handle_dollar                                        812     803      -9
parse_stream                                        2378    2356     -22
parse_redirect                                       408     372     -36
maybe_die                                             44       -     -44
is_assignment                                        215     134     -81
------------------------------------------------------------------------------
(add/remove: 4/1 grow/shrink: 4/8 up/down: 248/-208)           Total: 40 bytes

5 files changed, 214 insertions, 157 deletions

diff --git a/shell/hush.c b/shell/hush.c
index 4c2716ddd..41d65ff69 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -65,8 +65,6 @@
  * Licensed under the GPL v2 or later, see the file LICENSE in this tarball.
  */
 #include "busybox.h" /* for APPLET_IS_NOFORK/NOEXEC */
-//TODO: pull in some .h and find out whether we have SINGLE_APPLET_MAIN?
-//#include "applet_tables.h" doesn't work
 #include <glob.h>
 /* #include <dmalloc.h> */
 #if ENABLE_HUSH_CASE
@@ -80,11 +78,18 @@
 
 
 /* Debug build knobs */
-//#define LEAK_HUNTING 1
-//#define WANT_TO_TEST_NOMMU 1
+#define LEAK_HUNTING 0
+#define BUILD_AS_NOMMU 0
+/* Enable/disable sanity checks. Ok to enable in production,
+ * only adds a bit of bloat. Set to >1 to get non-production level verbosity.
+ * Keeping 1 for now even in released versions.
+ */
+#define HUSH_DEBUG 1
+/* In progress... */
+#define ENABLE_HUSH_FUNCTIONS 0
 
 
-#ifdef WANT_TO_TEST_NOMMU
+#if BUILD_AS_NOMMU
 # undef BB_MMU
 # undef USE_FOR_NOMMU
 # undef USE_FOR_MMU
@@ -122,15 +127,6 @@
 #define IF_HAS_NO_KEYWORDS(...) __VA_ARGS__
 #endif
 
-/* Enable/disable sanity checks. Ok to enable in production,
- * only adds a bit of bloat.
- * Keeping unconditionally on for now.
- */
-#define HUSH_DEBUG 1
-/* In progress... */
-#define ENABLE_HUSH_FUNCTIONS 0
-
-
 /* If you comment out one of these below, it will be #defined later
  * to perform debug printfs to stderr: */
 #define debug_printf(...)        do {} while (0)
@@ -216,40 +212,6 @@ static void debug_print_strings(const char *prefix, char **vv)
 #define debug_print_strings(prefix, vv) ((void)0)
 #endif
 
-/*
- * Leak hunting. Use hush_leaktool.sh for post-processing.
- */
-#ifdef LEAK_HUNTING
-static void *xxmalloc(int lineno, size_t size)
-{
-	void *ptr = xmalloc((size + 0xff) & ~0xff);
-	fdprintf(2, "line %d: malloc %p\n", lineno, ptr);
-	return ptr;
-}
-static void *xxrealloc(int lineno, void *ptr, size_t size)
-{
-	ptr = xrealloc(ptr, (size + 0xff) & ~0xff);
-	fdprintf(2, "line %d: realloc %p\n", lineno, ptr);
-	return ptr;
-}
-static char *xxstrdup(int lineno, const char *str)
-{
-	char *ptr = xstrdup(str);
-	fdprintf(2, "line %d: strdup %p\n", lineno, ptr);
-	return ptr;
-}
-static void xxfree(void *ptr)
-{
-	fdprintf(2, "free %p\n", ptr);
-	free(ptr);
-}
-#define xmalloc(s)     xxmalloc(__LINE__, s)
-#define xrealloc(p, s) xxrealloc(__LINE__, p, s)
-#define xstrdup(s)     xxstrdup(__LINE__, s)
-#define free(p)        xxfree(p)
-#endif
-
-
 #define ERR_PTR ((void*)(long)1)
 
 #define JOB_STATUS_FORMAT "[%d] %-22s %.40s\n"
@@ -662,28 +624,100 @@ static const struct built_in_command bltins[] = {
 };
 
 
-static void maybe_die(const char *notice, const char *msg)
+/* Leak hunting. Use hush_leaktool.sh for post-processing.
+ */
+#if LEAK_HUNTING
+static void *xxmalloc(int lineno, size_t size)
+{
+	void *ptr = xmalloc((size + 0xff) & ~0xff);
+	fdprintf(2, "line %d: malloc %p\n", lineno, ptr);
+	return ptr;
+}
+static void *xxrealloc(int lineno, void *ptr, size_t size)
+{
+	ptr = xrealloc(ptr, (size + 0xff) & ~0xff);
+	fdprintf(2, "line %d: realloc %p\n", lineno, ptr);
+	return ptr;
+}
+static char *xxstrdup(int lineno, const char *str)
+{
+	char *ptr = xstrdup(str);
+	fdprintf(2, "line %d: strdup %p\n", lineno, ptr);
+	return ptr;
+}
+static void xxfree(void *ptr)
+{
+	fdprintf(2, "free %p\n", ptr);
+	free(ptr);
+}
+#define xmalloc(s)     xxmalloc(__LINE__, s)
+#define xrealloc(p, s) xxrealloc(__LINE__, p, s)
+#define xstrdup(s)     xxstrdup(__LINE__, s)
+#define free(p)        xxfree(p)
+#endif
+
+
+/* Syntax and runtime errors. They always abort scripts.
+ * In interactive use they usually discard unparsed and/or unexecuted commands
+ * and return to the prompt.
+ * HUSH_DEBUG >= 2 prints line number in this file where it was detected.
+ */
+#if HUSH_DEBUG < 2
+# define die_if_script(lineno, fmt, msg)       die_if_script(fmt, msg)
+# define syntax_error(lineno, msg)             syntax_error(msg)
+# define syntax_error_at(lineno, msg)          syntax_error_at(msg)
+# define syntax_error_unterminated(lineno, ch) syntax_error_unterminated(ch)
+#endif
+
+static void die_if_script(unsigned lineno, const char *fmt, const char *msg)
 {
-	/* Was using fancy stuff:
-	 * (G_interactive_fd ? bb_error_msg : bb_error_msg_and_die)(...params...)
-	 * but it SEGVs. ?! Oh well... explicit temp ptr works around that */
 	void FAST_FUNC (*fp)(const char *s, ...) = bb_error_msg_and_die;
 #if ENABLE_HUSH_INTERACTIVE
 	if (G_interactive_fd)
 		fp = bb_error_msg;
 #endif
-	fp(msg ? "%s: %s" : notice, notice, msg);
+#if HUSH_DEBUG >= 2
+	bb_error_msg("hush.c:%u", lineno);
+#endif
+	fp(fmt, msg);
+}
+
+static void syntax_error(unsigned lineno, const char *msg)
+{
+	if (msg)
+		die_if_script(lineno, "syntax error: %s", msg);
+	else
+		die_if_script(lineno, "syntax error", NULL);
+}
+
+static void syntax_error_at(unsigned lineno, const char *msg)
+{
+	die_if_script(lineno, "syntax error at '%s'", msg);
+}
+
+static void syntax_error_unterminated(unsigned lineno, char ch)
+{
+	char msg[2];
+	msg[0] = ch;
+	msg[1] = '\0';
+	die_if_script(lineno, "syntax error: unterminated %s", msg);
 }
-#if 1
-#define syntax(msg) maybe_die("syntax error", msg);
+
+#if HUSH_DEBUG < 2
+# undef die_if_script
+# undef syntax_error
+# undef syntax_error_at
+# undef syntax_error_unterminated
 #else
-/* Debug -- trick gcc to expand __LINE__ and convert to string */
-#define __syntax(msg, line) maybe_die("syntax error hush.c:" # line, msg)
-#define _syntax(msg, line) __syntax(msg, line)
-#define syntax(msg) _syntax(msg, __LINE__)
+# define die_if_script(fmt, msg)       die_if_script(__LINE__, fmt, msg)
+# define syntax_error(msg)             syntax_error(__LINE__, msg)
+# define syntax_error_at(msg)          syntax_error_at(__LINE__, msg)
+# define syntax_error_unterminated(ch) syntax_error_unterminated(__LINE__, ch)
 #endif
 
 
+/* Utility functions
+ */
 static int glob_needed(const char *s)
 {
 	while (*s) {
@@ -696,14 +730,14 @@ static int glob_needed(const char *s)
 	return 0;
 }
 
-static int is_assignment(const char *s)
+static int is_well_formed_var_name(const char *s, char terminator)
 {
 	if (!s || !(isalpha(*s) || *s == '_'))
 		return 0;
 	s++;
 	while (isalnum(*s) || *s == '_')
 		s++;
-	return *s == '=';
+	return *s == terminator;
 }
 
 /* Replace each \x with x in place, return ptr past NUL. */
@@ -747,7 +781,7 @@ static char **add_strings_to_strings(char **strings, char **add, int need_to_dup
 		v[count1 + i] = (need_to_dup ? xstrdup(add[i]) : add[i]);
 	return v;
 }
-#ifdef LEAK_HUNTING
+#if LEAK_HUNTING
 static char **xx_add_strings_to_strings(int lineno, char **strings, char **add, int need_to_dup)
 {
 	char **ptr = add_strings_to_strings(strings, add, need_to_dup);
@@ -765,7 +799,7 @@ static char **add_string_to_strings(char **strings, char *add)
 	v[1] = NULL;
 	return add_strings_to_strings(strings, v, /*dup:*/ 0);
 }
-#ifdef LEAK_HUNTING
+#if LEAK_HUNTING
 static char **xx_add_string_to_strings(int lineno, char **strings, char *add)
 {
 	char **ptr = add_string_to_strings(strings, add);
@@ -1911,12 +1945,19 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
 			free(exp_str);
 
 			if (errcode < 0) {
+				const char *msg = "error in arithmetic";
 				switch (errcode) {
-				case -3: maybe_die("arith", "exponent less than 0"); break;
-				case -2: maybe_die("arith", "divide by zero"); break;
-				case -5: maybe_die("arith", "expression recursion loop detected"); break;
-				default: maybe_die("arith", "syntax error"); break;
+				case -3:
+					msg = "exponent less than 0";
+					break;
+				case -2:
+					msg = "divide by 0";
+					break;
+				case -5:
+					msg = "expression recursion loop detected";
+					break;
 				}
+				die_if_script(msg, NULL);
 			}
 			debug_printf_subst("ARITH RES '"arith_t_fmt"'\n", res);
 			sprintf(arith_buf, arith_t_fmt, res);
@@ -1951,7 +1992,8 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
 					exp_save = var[exp_off];
 					exp_null = exp_save == ':';
 					exp_word = var + exp_off;
-					if (exp_null) ++exp_word;
+					if (exp_null)
+						++exp_word;
 					exp_op = *exp_word++;
 					var[exp_off] = '\0';
 				}
@@ -1995,17 +2037,28 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
 						exp_null ? "true" : "false", exp_test);
 					if (exp_test) {
 						if (exp_op == '?')
-							maybe_die(var, *exp_word ? exp_word : "parameter null or not set");
+//TODO: what does interactive bash
+							/* ${var?[error_msg_if_unset]} */
+							/* ${var:?[error_msg_if_unset_or_null]} */
+							/* mimic bash message */
+							if (*exp_word) {
+								char *msg = xasprintf("%s: %s", var, exp_word);
+								die_if_script("%s", msg);
+								free(msg);
+							} else {
+								die_if_script("%s: parameter null or not set", var);
+							}
 						else
 							val = exp_word;
 
 						if (exp_op == '=') {
+							/* ${var=[word]} or ${var:=[word]} */
 							if (isdigit(var[0]) || var[0] == '#') {
-								maybe_die(var, "special vars cannot assign in this way");
+								/* mimic bash message */
+								die_if_script("$%s: cannot assign in this way", var);
 								val = NULL;
 							} else {
-								char *new_var = xmalloc(strlen(var) + strlen(val) + 2);
-								sprintf(new_var, "%s=%s", var, val);
+								char *new_var = xasprintf("%s=%s", var, val);
 								set_local_var(new_var, -1, 0);
 							}
 						}
@@ -3280,7 +3333,7 @@ static int run_list(struct pipe *pi)
 			continue;
 		/* current word is FOR or IN (BOLD in comments below) */
 		if (cpipe->next == NULL) {
-			syntax("malformed for");
+			syntax_error("malformed for");
 			debug_printf_exec("run_list lvl %d return 1\n", G.run_list_level);
 			return 1;
 		}
@@ -3291,7 +3344,7 @@ static int run_list(struct pipe *pi)
 		if (cpipe->res_word == RES_IN /* "for v IN a b; not_do..."? */
 		 || cpipe->next->res_word != RES_IN /* FOR v not_do_and_not_in..."? */
 		) {
-			syntax("malformed for");
+			syntax_error("malformed for");
 			debug_printf_exec("run_list lvl %d return 1\n", G.run_list_level);
 			return 1;
 		}
@@ -3443,7 +3496,7 @@ static int run_list(struct pipe *pi)
 				break;
 			}
 			/* Insert next value from for_lcur */
-//TODO: does it need escaping?
+			/* note: *for_lcur already has quotes removed, $var expanded, etc */
 			pi->cmds[0].argv[0] = xasprintf("%s=%s", for_varname, *for_lcur++);
 			pi->cmds[0].assignment_cnt = 1;
 		}
@@ -3846,7 +3899,7 @@ static int reserved_word(o_string *word, struct parse_context *ctx)
 #endif
 	if (r->flag == 0) { /* '!' */
 		if (ctx->ctx_inverted) { /* bash doesn't accept '! ! true' */
-			syntax("! ! command");
+			syntax_error("! ! command");
 			IF_HAS_KEYWORDS(ctx->ctx_res_w = RES_SNTX;)
 		}
 		ctx->ctx_inverted = 1;
@@ -3860,7 +3913,7 @@ static int reserved_word(o_string *word, struct parse_context *ctx)
 		initialize_context(ctx);
 		ctx->stack = old;
 	} else if (/*ctx->ctx_res_w == RES_NONE ||*/ !(ctx->old_flag & (1 << r->res))) {
-		syntax(word->data);
+		syntax_error_at(word->data);
 		ctx->ctx_res_w = RES_SNTX;
 		return 1;
 	}
@@ -3943,7 +3996,7 @@ static int done_word(o_string *word, struct parse_context *ctx)
 		 * while if false; then false; fi; do; break; done
 		 * TODO? */
 		if (command->group) {
-			syntax(word->data);
+			syntax_error_at(word->data);
 			debug_printf_parse("done_word return 1: syntax error, "
 					"groups and arglists don't mix\n");
 			return 1;
@@ -4008,7 +4061,10 @@ static int done_word(o_string *word, struct parse_context *ctx)
 	 * as it is "for v; in ...". FOR and IN become two pipe structs
 	 * in parse tree. */
 	if (ctx->ctx_res_w == RES_FOR) {
-//TODO: check that command->argv[0] is a valid variable name!
+		if (!is_well_formed_var_name(command->argv[0], '\0')) {
+			syntax_error("malformed variable name in for");
+			return 1;
+		}
 		done_pipe(ctx, PIPE_SEQ);
 	}
 #endif
@@ -4091,12 +4147,6 @@ static int parse_redirect(struct parse_context *ctx,
 			nommu_addchr(&ctx->as_string, ch);
 			ch = i_peek(input);
 		}
-		/* <<[-] word is the same as <<[-]word */
-		while (ch == ' ' || ch == '\t') {
-			ch = i_getch(input);
-			nommu_addchr(&ctx->as_string, ch);
-			ch = i_peek(input);
-		}
 	}
 
 	if (style == REDIRECT_OVERWRITE && dup_num == -1) {
@@ -4222,7 +4272,7 @@ static int fetch_heredocs(int heredoc_cnt, struct parse_context *ctx, struct in_
 {
 	struct pipe *pi = ctx->list_head;
 
-	while (pi) {
+	while (pi && heredoc_cnt) {
 		int i;
 		struct command *cmd = pi->cmds;
 
@@ -4238,16 +4288,12 @@ static int fetch_heredocs(int heredoc_cnt, struct parse_context *ctx, struct in_
 				if (redir->rd_type == REDIRECT_HEREDOC) {
 					char *p;
 
-					if (heredoc_cnt <= 0) {
-						syntax("heredoc BUG 1");
-						return 1; /* error */
-					}
 					redir->rd_type = REDIRECT_HEREDOC2;
 					/* redir->dup is (ab)used to indicate <<- */
 					p = fetch_till_str(&ctx->as_string, input,
 						redir->rd_filename, redir->rd_dup & HEREDOC_SKIPTABS);
 					if (!p) {
-						syntax("unexpected EOF in here document");
+						syntax_error("unexpected EOF in here document");
 						return 1;
 					}
 					free(redir->rd_filename);
@@ -4260,10 +4306,12 @@ static int fetch_heredocs(int heredoc_cnt, struct parse_context *ctx, struct in_
 		}
 		pi = pi->next;
 	}
+#if 0
 	/* Should be 0. If it isn't, it's a parse error */
 	if (heredoc_cnt)
-		syntax("heredoc BUG 2");
-	return heredoc_cnt;
+		bb_error_msg_and_die("heredoc BUG 2");
+#endif
+	return 0;
 }
 
 
@@ -4388,7 +4436,7 @@ static int parse_group(o_string *dest, struct parse_context *ctx,
 	 || dest->length /* word(... */
 	 || dest->o_quoted /* ""(... */
 	) {
-		syntax(NULL);
+		syntax_error(NULL);
 		debug_printf_parse("parse_group return 1: "
 			"syntax error, groups and arglists don't mix\n");
 		return 1;
@@ -4412,7 +4460,7 @@ static int parse_group(o_string *dest, struct parse_context *ctx,
 #if !BB_MMU
 			free(as_string);
 #endif
-			syntax(NULL);
+			syntax_error(NULL);
 			debug_printf_parse("parse_group return 1: "
 				"parse_stream returned %p\n", pipe_list);
 			return 1;
@@ -4439,7 +4487,7 @@ static int add_till_single_quote(o_string *dest, struct in_str *input)
 	while (1) {
 		int ch = i_getch(input);
 		if (ch == EOF) {
-			syntax("unterminated '");
+			syntax_error_unterminated('\'');
 			return 1;
 		}
 		if (ch == '\'')
@@ -4453,7 +4501,7 @@ static int add_till_double_quote(o_string *dest, struct in_str *input)
 	while (1) {
 		int ch = i_getch(input);
 		if (ch == EOF) {
-			syntax("unterminated \"");
+			syntax_error_unterminated('"');
 			return 1;
 		}
 		if (ch == '"')
@@ -4491,7 +4539,7 @@ static int add_till_backquote(o_string *dest, struct in_str *input)
 	while (1) {
 		int ch = i_getch(input);
 		if (ch == EOF) {
-			syntax("unterminated `");
+			syntax_error_unterminated('`');
 			return 1;
 		}
 		if (ch == '`')
@@ -4500,7 +4548,7 @@ static int add_till_backquote(o_string *dest, struct in_str *input)
 			/* \x. Copy both chars unless it is \` */
 			int ch2 = i_getch(input);
 			if (ch2 == EOF) {
-				syntax("unterminated `");
+				syntax_error_unterminated('`');
 				return 1;
 			}
 			if (ch2 != '`' && ch2 != '$' && ch2 != '\\')
@@ -4528,7 +4576,7 @@ static int add_till_closing_paren(o_string *dest, struct in_str *input, bool dbl
 	while (1) {
 		int ch = i_getch(input);
 		if (ch == EOF) {
-			syntax("unterminated )");
+			syntax_error_unterminated(')');
 			return 1;
 		}
 		if (ch == '(')
@@ -4560,7 +4608,7 @@ static int add_till_closing_paren(o_string *dest, struct in_str *input, bool dbl
 			/* \x. Copy verbatim. Important for  \(, \) */
 			ch = i_getch(input);
 			if (ch == EOF) {
-				syntax("unterminated )");
+				syntax_error_unterminated(')');
 				return 1;
 			}
 			o_addchr(dest, ch);
@@ -4678,7 +4726,7 @@ static int handle_dollar(o_string *as_string,
 					break;
 				default:
 				case_default:
-					syntax("unterminated ${name}");
+					syntax_error("unterminated ${name}");
 					debug_printf_parse("handle_dollar return 1: unterminated ${name}\n");
 					return 1;
 				}
@@ -4783,7 +4831,7 @@ static int parse_stream_dquoted(o_string *as_string,
 	}
 	/* note: can't move it above ch == dquote_end check! */
 	if (ch == EOF) {
-		syntax("unterminated \"");
+		syntax_error_unterminated('"');
 		debug_printf_parse("parse_stream_dquoted return 1: unterminated \"\n");
 		return 1;
 	}
@@ -4794,8 +4842,9 @@ static int parse_stream_dquoted(o_string *as_string,
 	debug_printf_parse(": ch=%c (%d) escape=%d\n",
 					ch, ch, dest->o_escape);
 	if (ch == '\\') {
+//TODO: check interactive behavior
 		if (next == EOF) {
-			syntax("\\<eof>");
+			syntax_error("\\<eof>");
 			debug_printf_parse("parse_stream_dquoted return 1: \\<eof>\n");
 			return 1;
 		}
@@ -4839,7 +4888,7 @@ static int parse_stream_dquoted(o_string *as_string,
 	if (ch == '='
 	 && (dest->o_assignment == MAYBE_ASSIGNMENT
 	    || dest->o_assignment == WORD_IS_KEYWORD)
-	 && is_assignment(dest->data)
+	 && is_well_formed_var_name(dest->data, '=')
 	) {
 		dest->o_assignment = DEFINITELY_ASSIGNMENT;
 	}
@@ -4905,7 +4954,7 @@ static struct pipe *parse_stream(char **pstring,
 			struct pipe *pi;
 
 			if (heredoc_cnt) {
-				syntax("unterminated here document");
+				syntax_error("unterminated here document");
 				goto parse_error;
 			}
 			if (done_word(&dest, &ctx)) {
@@ -4943,7 +4992,7 @@ static struct pipe *parse_stream(char **pstring,
 			if ((dest.o_assignment == MAYBE_ASSIGNMENT
 			    || dest.o_assignment == WORD_IS_KEYWORD)
 			 && ch == '='
-			 && is_assignment(dest.data)
+			 && is_well_formed_var_name(dest.data, '=')
 			) {
 				dest.o_assignment = DEFINITELY_ASSIGNMENT;
 			}
@@ -4994,7 +5043,7 @@ static struct pipe *parse_stream(char **pstring,
 				 * We require heredoc to be in enclosing {}/(),
 				 * if any.
 				 */
-				syntax("unterminated here document");
+				syntax_error("unterminated here document");
 				goto parse_error;
 			}
 			if (done_word(&dest, &ctx)) {
@@ -5051,7 +5100,7 @@ static struct pipe *parse_stream(char **pstring,
 			break;
 		case '\\':
 			if (next == EOF) {
-				syntax("\\<eof>");
+				syntax_error("\\<eof>");
 				goto parse_error;
 			}
 			o_addchr(&dest, '\\');
@@ -5074,7 +5123,7 @@ static struct pipe *parse_stream(char **pstring,
 			while (1) {
 				ch = i_getch(input);
 				if (ch == EOF) {
-					syntax("unterminated '");
+					syntax_error_unterminated('\'');
 					goto parse_error;
 				}
 				nommu_addchr(&ctx.as_string, ch);
@@ -5126,7 +5175,7 @@ static struct pipe *parse_stream(char **pstring,
 			}
 #if 0
 			else if (next == '(') {
-				syntax(">(process) not supported");
+				syntax_error(">(process) not supported");
 				goto parse_error;
 			}
 #endif
@@ -5152,7 +5201,7 @@ static struct pipe *parse_stream(char **pstring,
 			}
 #if 0
 			else if (next == '(') {
-				syntax("<(process) not supported");
+				syntax_error("<(process) not supported");
 				goto parse_error;
 			}
 #endif
@@ -5246,7 +5295,7 @@ static struct pipe *parse_stream(char **pstring,
 					ch = i_getch(input);
 				} while (ch == ' ' || ch == '\n');
 				if (ch != '{') {
-					syntax("was expecting {");
+					syntax_error("was expecting {");
 					goto parse_error;
 				}
 				ch = 'F'; /* magic value */
@@ -5266,7 +5315,7 @@ static struct pipe *parse_stream(char **pstring,
 			/* proper use of this character is caught by end_trigger:
 			 * if we see {, we call parse_group(..., end_trigger='}')
 			 * and it will match } earlier (not here). */
-			syntax("unexpected } or )");
+			syntax_error("unexpected } or )");
 			goto parse_error;
 		default:
 			if (HUSH_DEBUG)
@@ -6111,8 +6160,16 @@ static int builtin_pwd(char **argv UNUSED_PARAM)
 static int builtin_read(char **argv)
 {
 	char *string;
-	const char *name = argv[1] ? argv[1] : "REPLY";
-//TODO: check that argv[1] is a valid variable name
+	const char *name = "REPLY";
+
+	if (argv[1]) {
+		name = argv[1];
+		if (!is_well_formed_var_name(name, '\0')) {
+			/* Mimic bash message */
+			bb_error_msg("read: '%s': not a valid identifier", name);
+			return 1;
+		}
+	}
 
 	string = xmalloc_reads(STDIN_FILENO, xasprintf("%s=", name), NULL);
 	return set_local_var(string, 0, 0);
diff --git a/shell/hush_test/hush-arith/arith.right b/shell/hush_test/hush-arith/arith.right
index a35fe893f..8cde0ee53 100644
--- a/shell/hush_test/hush-arith/arith.right
+++ b/shell/hush_test/hush-arith/arith.right
@@ -55,28 +55,28 @@ Format: 'expected actual'
 30 30
 20 20
 30 30
-hush: arith: syntax error
+hush: error in arithmetic
 6 6
 6,5,3 6,5,3
 263 263
 255 255
 40 40
-hush: arith: syntax error
-hush: arith: divide by zero
+hush: error in arithmetic
+hush: divide by 0
 hush: can't exec 'let': No such file or directory
-hush: arith: syntax error
+hush: error in arithmetic
 hush: can't exec 'let': No such file or directory
 abc
 def
 ghi
-hush: arith: syntax error
+hush: error in arithmetic
 16 16
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
 9 9
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
 9 9
 9 9
 9 9
@@ -97,18 +97,18 @@ hush: arith: syntax error
 3 3
 4 4
 4 4
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
 4 4
 7 7
 -7 -7
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
 6 6
 3 3
 7 7
@@ -119,19 +119,19 @@ hush: arith: syntax error
 2 2
 -2 -2
 1 1
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
+hush: error in arithmetic
 5 5
 1 1
 4 4
 0 0
-hush: arith: syntax error
-hush: arith: syntax error
+hush: error in arithmetic
+hush: error in arithmetic
 8 12
-hush: arith: syntax error
+hush: error in arithmetic
 42
 42
 42
diff --git a/shell/hush_test/hush-vars/param_expand_assign.right b/shell/hush_test/hush-vars/param_expand_assign.right
index fff4ead33..d5b258073 100644
--- a/shell/hush_test/hush-vars/param_expand_assign.right
+++ b/shell/hush_test/hush-vars/param_expand_assign.right
@@ -2,10 +2,10 @@ hush: syntax error: unterminated ${name}
 hush: syntax error: unterminated ${name}
 0
 0
-hush: 1: special vars cannot assign in this way
-hush: 1: special vars cannot assign in this way
-hush: 1: special vars cannot assign in this way
-hush: 1: special vars cannot assign in this way
+hush: $1: cannot assign in this way
+hush: $1: cannot assign in this way
+hush: $1: cannot assign in this way
+hush: $1: cannot assign in this way
 _aa
 _aa
 _aa
diff --git a/shell/hush_test/hush-vars/param_expand_indicate_error.right b/shell/hush_test/hush-vars/param_expand_indicate_error.right
index f440f6fae..ec4908c35 100644
--- a/shell/hush_test/hush-vars/param_expand_indicate_error.right
+++ b/shell/hush_test/hush-vars/param_expand_indicate_error.right
@@ -5,8 +5,8 @@ hush: syntax error: unterminated ${name}
 _
 hush: 1: parameter null or not set
 hush: 1: parameter null or not set
-hush: 1: word
-hush: 1: word
+hush: 1: message1
+hush: 1: message1
 _aaaa
 _aaaa
 _aaaa
@@ -15,13 +15,13 @@ _aaaa
 _
 hush: f: parameter null or not set
 hush: f: parameter null or not set
-hush: f: word
-hush: f: word
+hush: f: message3
+hush: f: message3
 _
 _
 hush: f: parameter null or not set
 _
-hush: f: word
+hush: f: message4
 _fff
 _fff
 _fff
diff --git a/shell/hush_test/hush-vars/param_expand_indicate_error.tests b/shell/hush_test/hush-vars/param_expand_indicate_error.tests
index 77834fedd..1f94181a9 100755
--- a/shell/hush_test/hush-vars/param_expand_indicate_error.tests
+++ b/shell/hush_test/hush-vars/param_expand_indicate_error.tests
@@ -12,8 +12,8 @@
 "$THIS_SH" -c 'set --; echo _$1'
 "$THIS_SH" -c 'set --; echo _${1?}'
 "$THIS_SH" -c 'set --; echo _${1:?}'
-"$THIS_SH" -c 'set --; echo _${1?word}'
-"$THIS_SH" -c 'set --; echo _${1:?word}'
+"$THIS_SH" -c 'set --; echo _${1?message1}'
+"$THIS_SH" -c 'set --; echo _${1:?message1}'
 
 "$THIS_SH" -c 'set -- aaaa; echo _$1'
 "$THIS_SH" -c 'set -- aaaa; echo _${1?}'
@@ -24,14 +24,14 @@
 "$THIS_SH" -c 'unset f; echo _$f'
 "$THIS_SH" -c 'unset f; echo _${f?}'
 "$THIS_SH" -c 'unset f; echo _${f:?}'
-"$THIS_SH" -c 'unset f; echo _${f?word}'
-"$THIS_SH" -c 'unset f; echo _${f:?word}'
+"$THIS_SH" -c 'unset f; echo _${f?message3}'
+"$THIS_SH" -c 'unset f; echo _${f:?message3}'
 
 "$THIS_SH" -c 'f=; echo _$f'
 "$THIS_SH" -c 'f=; echo _${f?}'
 "$THIS_SH" -c 'f=; echo _${f:?}'
 "$THIS_SH" -c 'f=; echo _${f?word}'
-"$THIS_SH" -c 'f=; echo _${f:?word}'
+"$THIS_SH" -c 'f=; echo _${f:?message4}'
 
 "$THIS_SH" -c 'f=fff; echo _$f'
 "$THIS_SH" -c 'f=fff; echo _${f?}'