hush patches

hush patches

[Sascha Hauer]

This contains several hush cleanup patches I came across while trying
to fix the weird behaviour that an interactive hush currently exits
on a syntax error. This is fixed and also we now get a nice
'syntax error: unexpected token `x'' in some cases. Hopefully no other
functional changes introduced.

----------------------------------------------------------------
Sascha Hauer (10):
      hush: fix exit on syntax error behaviour
      hush: refactor readline call
      readline: remove unused variable
      hush: refactor get_user_input()
      hush: refactor file_get()
      hush: rename __promptme to interrupt
      consolidate syntax() and syntax_err()
      hush: Be more informative on syntax error
      hush: use standard ARRAY_SIZE
      hush: refactor reserved_word()

 common/hush.c  | 188 ++++++++++++++++++++++++++++++++-------------------------
 lib/readline.c |   3 +-
 2 files changed, 108 insertions(+), 83 deletions(-)

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/10] hush: fix exit on syntax error behaviour

[Sascha Hauer]

input.__promptme is no valid indicator that run_shell should be left.
It should be left on executing the 'exit' builtin which is indicated
by a return code < 0 from parse_stream_outer(). Track this with an extra
variable and use it as a condition to return from an interactive shell.

This fixes the weird behaviour that hush exits (and the user finds itself
in the menu) when a syntax error occured.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index bf1d9e6..5969127 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1853,14 +1853,17 @@ int run_shell(void)
 	int rcode;
 	struct in_str input;
 	struct p_context ctx;
+	int exit = 0;
 
 	do {
 		setup_file_in_str(&input);
 		rcode = parse_stream_outer(&ctx, &input, FLAG_PARSE_SEMICOLON);
-		if (rcode < -1)
+		if (rcode < -1) {
+			exit = 1;
 			rcode = -rcode - 2;
+		}
 		release_context(&ctx);
-	} while (!input.__promptme);
+	} while (!exit);
 
 	return rcode;
 }
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/10] hush: refactor readline call

[Sascha Hauer]

Don't call readline in if/else, instead setup a variable and call
it once.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 5969127..0f1a9b9 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -420,15 +420,16 @@ static void get_user_input(struct in_str *i)
 {
 	int n;
 	static char the_command[CONFIG_CBSIZE];
+	char *prompt;
 
 	i->__promptme = 1;
 
-	if (i->promptmode == 1) {
-		n = readline(getprompt(), console_buffer, CONFIG_CBSIZE);
-	} else {
-		n = readline(CONFIG_PROMPT_HUSH_PS2, console_buffer, CONFIG_CBSIZE);
-	}
+	if (i->promptmode == 1)
+		prompt = getprompt();
+	else
+		prompt = CONFIG_PROMPT_HUSH_PS2;
 
+	n = readline(prompt, console_buffer, CONFIG_CBSIZE);
 	if (n == -1 ) {
 		i->__promptme = 0;
 		n = 0;
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/10] readline: remove unused variable

[Sascha Hauer]

'rc' is never set to anything else than 0, so drop it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 lib/readline.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/readline.c b/lib/readline.c
index 3fb620e..6afc491 100644
--- a/lib/readline.c
+++ b/lib/readline.c
@@ -191,7 +191,6 @@ int readline(const char *prompt, char *buf, int len)
 	unsigned wlen;
 	int ichar;
 	int insert = 1;
-	int rc = 0;
 #ifdef CONFIG_AUTO_COMPLETE
 	char tmp;
 	int reprint, i;
@@ -351,5 +350,5 @@ int readline(const char *prompt, char *buf, int len)
 		cread_add_to_hist(buf);
 	hist_cur = hist_add_idx;
 
-	return rc < 0 ? rc : len;
+	return len;
 }
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/10] hush: refactor get_user_input()

[Sascha Hauer]

Save indentation level by returning early.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 0f1a9b9..7a077ba 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -441,25 +441,27 @@ static void get_user_input(struct in_str *i)
 	if (i->promptmode == 1) {
 		strcpy(the_command,console_buffer);
 		i->p = the_command;
-	} else {
-		if (console_buffer[0] != '\n') {
-			if (strlen(the_command) + strlen(console_buffer)
-			    < CONFIG_CBSIZE) {
-				n = strlen(the_command);
-				the_command[n - 1] = ' ';
-				strcpy(&the_command[n], console_buffer);
-			}
-			else {
-				the_command[0] = '\n';
-				the_command[1] = '\0';
-			}
-		}
-		if (i->__promptme == 0) {
+		return;
+	}
+
+	if (console_buffer[0] != '\n') {
+		if (strlen(the_command) + strlen(console_buffer)
+		    < CONFIG_CBSIZE) {
+			n = strlen(the_command);
+			the_command[n - 1] = ' ';
+			strcpy(&the_command[n], console_buffer);
+		} else {
 			the_command[0] = '\n';
 			the_command[1] = '\0';
 		}
-		i->p = console_buffer;
 	}
+
+	if (i->__promptme == 0) {
+		the_command[0] = '\n';
+		the_command[1] = '\0';
+	}
+
+	i->p = console_buffer;
 }
 
 /* This is the magic location that prints prompts
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/10] hush: refactor file_get()

[Sascha Hauer]

Save indentation level by returning early.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 7a077ba..f0f2eda 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -471,21 +471,23 @@ static int file_get(struct in_str *i)
 	int ch;
 
 	ch = 0;
+
 	/* If there is data waiting, eat it up */
-	if (i->p && *i->p) {
+	if (i->p && *i->p)
+		return *i->p++;
+
+	/* need to double check i->file because we might be doing something
+	 * more complicated by now, like sourcing or substituting. */
+	while (!i->p  || strlen(i->p) == 0 )
+		get_user_input(i);
+
+	i->promptmode = 2;
+
+	if (i->p && *i->p)
 		ch = *i->p++;
-	} else {
-		/* need to double check i->file because we might be doing something
-		 * more complicated by now, like sourcing or substituting. */
-			while (!i->p  || strlen(i->p) == 0 ) {
-				get_user_input(i);
-			}
-			i->promptmode = 2;
-			if (i->p && *i->p) {
-				ch = *i->p++;
-			}
-		debug("%s: got a %d\n", __func__, ch);
-	}
+
+	debug("%s: got a %d\n", __func__, ch);
+
 	return ch;
 }
 
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/10] hush: rename __promptme to interrupt

[Sascha Hauer]

the name '__promptme' does not make clear what the variable means. rename
it to 'interrupt' which is set to true when the user has hit ctrl-c.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index f0f2eda..95054b3 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -254,7 +254,7 @@ typedef struct {
  * available?  Where is it documented? */
 struct in_str {
 	const char *p;
-	int __promptme;
+	int interrupt;
 	int promptmode;
 	int (*get) (struct in_str *);
 	int (*peek) (struct in_str *);
@@ -422,8 +422,6 @@ static void get_user_input(struct in_str *i)
 	static char the_command[CONFIG_CBSIZE];
 	char *prompt;
 
-	i->__promptme = 1;
-
 	if (i->promptmode == 1)
 		prompt = getprompt();
 	else
@@ -431,7 +429,7 @@ static void get_user_input(struct in_str *i)
 
 	n = readline(prompt, console_buffer, CONFIG_CBSIZE);
 	if (n == -1 ) {
-		i->__promptme = 0;
+		i->interrupt = 1;
 		n = 0;
 	}
 
@@ -456,7 +454,7 @@ static void get_user_input(struct in_str *i)
 		}
 	}
 
-	if (i->__promptme == 0) {
+	if (i->interrupt) {
 		the_command[0] = '\n';
 		the_command[1] = '\0';
 	}
@@ -503,7 +501,7 @@ static void setup_file_in_str(struct in_str *i)
 {
 	i->peek = file_peek;
 	i->get = file_get;
-	i->__promptme = 1;
+	i->interrupt = 0;
 	i->promptmode = 1;
 	i->p = NULL;
 }
@@ -512,7 +510,7 @@ static void setup_string_in_str(struct in_str *i, const char *s)
 {
 	i->peek = static_peek;
 	i->get = static_get;
-	i->__promptme = 1;
+	i->interrupt = 0;
 	i->promptmode = 1;
 	i->p = s;
 }
@@ -1470,7 +1468,7 @@ static int parse_stream(o_string *dest, struct p_context *ctx,
 
 	while ((ch = b_getch(input)) != EOF) {
 		m = map[ch];
-		if (input->__promptme == 0)
+		if (input->interrupt)
 			return 1;
 		next = (ch == '\n') ? 0 : b_peek(input);
 
@@ -1528,7 +1526,7 @@ static int parse_stream(o_string *dest, struct p_context *ctx,
 			dest->nonnull = 1;
 			b_addchr(dest, '\'');
 			while (ch = b_getch(input), ch != EOF && ch != '\'') {
-				if (input->__promptme == 0)
+				if (input->interrupt)
 					return 1;
 				b_addchr(dest,ch);
 			}
@@ -1660,7 +1658,7 @@ static int parse_stream_outer(struct p_context *ctx, struct in_str *inp, int fla
 				free(ctx->stack);
 				b_reset(&temp);
 			}
-			if (inp->__promptme == 0)
+			if (inp->interrupt)
 				printf("<INTERRUPT>\n");
 			temp.nonnull = 0;
 			temp.quote = 0;
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/10] consolidate syntax() and syntax_err()

[Sascha Hauer]

syntax is defined as syntax_err. Drop syntax_err and call syntax instead.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 95054b3..3c4d244 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -132,7 +132,6 @@ extern int do_bootd(int flag, int argc, char *argv[]);      /* do_bootd */
 
 #define EXIT_SUCCESS 0
 #define EOF -1
-#define syntax() syntax_err()
 #define xstrdup strdup
 #define error_msg printf
 
@@ -265,7 +264,7 @@ struct in_str {
 
 #define final_printf debug
 
-static void syntax_err(void) {
+static void syntax(void) {
 	 printf("syntax error\n");
 }
 
@@ -1551,7 +1550,7 @@ static int parse_stream(o_string *dest, struct p_context *ctx,
 				b_getch(input);
 				done_pipe(ctx, PIPE_AND);
 			} else {
-				syntax_err();
+				syntax();
 				return 1;
 			}
 			break;
@@ -1564,7 +1563,7 @@ static int parse_stream(o_string *dest, struct p_context *ctx,
 				/* we could pick up a file descriptor choice here
 				 * with redirect_opt_num(), but bash doesn't do it.
 				 * "echo foo 2| cat" yields "foo 2". */
-				syntax_err();
+				syntax();
 				return 1;
 			}
 			break;
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/10] hush: Be more informative on syntax error

[Sascha Hauer]

Print the token that led to a syntax error, at least for the common
case.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 3c4d244..35c3c3c 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -264,8 +264,27 @@ struct in_str {
 
 #define final_printf debug
 
-static void syntax(void) {
-	 printf("syntax error\n");
+static void syntax(void)
+{
+	printf("syntax error\n");
+}
+
+static void syntaxf(const char *fmt, ...)
+{
+	va_list args;
+
+	printf("syntax error: ");
+
+	va_start(args, fmt);
+
+	vprintf(fmt, args);
+
+	va_end(args);
+}
+
+static void syntax_unexpected_token(const char *token)
+{
+	syntaxf("unexpected token `%s'\n", token);
 }
 
 /*   o_string manipulation: */
@@ -1201,7 +1220,7 @@ static int reserved_word(o_string *dest, struct p_context *ctx)
 				initialize_context(ctx);
 				ctx->stack = new;
 			} else if (ctx->w == RES_NONE || !(ctx->old_flag & (1 << r->code))) {
-				syntax();
+				syntax_unexpected_token(r->literal);
 				ctx->w = RES_SNTX;
 				b_reset(dest);
 				return 1;
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/10] hush: use standard ARRAY_SIZE

[Sascha Hauer]

Use standard ARRAY_SIZE instead of handcrafted NRES.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 35c3c3c..d844e74 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1192,13 +1192,15 @@ static struct reserved_combo reserved_list[] = {
 	{ "do",    RES_DO,    FLAG_DONE },
 	{ "done",  RES_DONE,  FLAG_END  }
 };
-#define NRES (sizeof(reserved_list)/sizeof(struct reserved_combo))
 
 static int reserved_word(o_string *dest, struct p_context *ctx)
 {
 	struct reserved_combo *r;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(reserved_list); i++) {
+		r = &reserved_list[i];
 
-	for (r = reserved_list; r < reserved_list + NRES; r++) {
 		if (strcmp(dest->data, r->literal) == 0) {
 
 			debug("found reserved word %s, code %d\n",r->literal,r->code);
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/10] hush: refactor reserved_word()

[Sascha Hauer]

Save indentation level for easier readability.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/hush.c | 64 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index d844e74..abe2ced 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1201,52 +1201,52 @@ static int reserved_word(o_string *dest, struct p_context *ctx)
 	for (i = 0; i < ARRAY_SIZE(reserved_list); i++) {
 		r = &reserved_list[i];
 
-		if (strcmp(dest->data, r->literal) == 0) {
-
-			debug("found reserved word %s, code %d\n",r->literal,r->code);
+		if (strcmp(dest->data, r->literal))
+			continue;
 
-			if (r->flag & FLAG_START) {
-				struct p_context *new = xmalloc(sizeof(struct p_context));
+		debug("found reserved word %s, code %d\n",r->literal,r->code);
 
-				debug("push stack\n");
+		if (r->flag & FLAG_START) {
+			struct p_context *new = xmalloc(sizeof(struct p_context));
 
-				if (ctx->w == RES_IN || ctx->w == RES_FOR) {
-					syntax();
-					free(new);
-					ctx->w = RES_SNTX;
-					b_reset(dest);
+			debug("push stack\n");
 
-					return 1;
-				}
-				*new = *ctx;   /* physical copy */
-				initialize_context(ctx);
-				ctx->stack = new;
-			} else if (ctx->w == RES_NONE || !(ctx->old_flag & (1 << r->code))) {
-				syntax_unexpected_token(r->literal);
+			if (ctx->w == RES_IN || ctx->w == RES_FOR) {
+				syntax();
+				free(new);
 				ctx->w = RES_SNTX;
 				b_reset(dest);
+
 				return 1;
 			}
+			*new = *ctx;   /* physical copy */
+			initialize_context(ctx);
+			ctx->stack = new;
+		} else if (ctx->w == RES_NONE || !(ctx->old_flag & (1 << r->code))) {
+			syntax_unexpected_token(r->literal);
+			ctx->w = RES_SNTX;
+			b_reset(dest);
+			return 1;
+		}
 
-			ctx->w = r->code;
-			ctx->old_flag = r->flag;
+		ctx->w = r->code;
+		ctx->old_flag = r->flag;
 
-			if (ctx->old_flag & FLAG_END) {
-				struct p_context *old;
+		if (ctx->old_flag & FLAG_END) {
+			struct p_context *old;
 
-				debug("pop stack\n");
+			debug("pop stack\n");
 
-				done_pipe(ctx,PIPE_SEQ);
-				old = ctx->stack;
-				old->child->group = ctx->list_head;
-				*ctx = *old;   /* physical copy */
-				free(old);
-			}
+			done_pipe(ctx,PIPE_SEQ);
+			old = ctx->stack;
+			old->child->group = ctx->list_head;
+			*ctx = *old;   /* physical copy */
+			free(old);
+		}
 
-			b_reset(dest);
+		b_reset(dest);
 
-			return 1;
-		}
+		return 1;
 	}
 
 	return 0;
-- 
1.8.4.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox