Patchwork [U-Boot,1/2] hush: Treat trailing || and && as incomplete statements

login
register
mail settings
Submitter Joe Hershberger
Date Nov. 8, 2012, 8:15 p.m.
Message ID <1352405756-401-1-git-send-email-joe.hershberger@ni.com>
Download mbox | patch
Permalink /patch/197877/
State Rejected
Delegated to: Tom Rini
Headers show

Comments

Joe Hershberger - Nov. 8, 2012, 8:15 p.m.
A || or && at the end of a command should behave just like an if
statment that is not complete.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/hush.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Wolfgang Denk - Nov. 8, 2012, 11:04 p.m.
Dear Joe,

In message <1352405756-401-1-git-send-email-joe.hershberger@ni.com> you wrote:
> A || or && at the end of a command should behave just like an if
> statment that is not complete.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

I really appreciate your patches.  But after having spent some time on
hush bug fixes myself (I submitted a few patches some time ago - to
kill them later when I learned I was just chiseling at the top of
somce iceberg) I seriously doubt if it makes senxe to try fixing the
many, many bugs in the old version of hush that is included with
U-Boot.

You may have a look over the fence at what Barebox fixed in this area,
but the result will probably be the same.

I fear that such patches, while fixing a nasty problem here and there,
will create other and maybe even more nasty problems in unexpacted
areas.


My suggestion is to save such efforts, and rather spend the time on
adating a more recent version of hush to U-Boot; getting things like
shell functions or backtic evaluation would be more than welcome to
many of us, I guess.

Best regards,

Wolfgang Denk
Joe Hershberger - Nov. 8, 2012, 11:26 p.m.
Hi Wolfgang,

On Thu, Nov 8, 2012 at 5:04 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Joe,
>
> In message <1352405756-401-1-git-send-email-joe.hershberger@ni.com> you wrote:
>> A || or && at the end of a command should behave just like an if
>> statment that is not complete.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> I really appreciate your patches.  But after having spent some time on
> hush bug fixes myself (I submitted a few patches some time ago - to
> kill them later when I learned I was just chiseling at the top of
> somce iceberg) I seriously doubt if it makes senxe to try fixing the
> many, many bugs in the old version of hush that is included with
> U-Boot.

I'm not attempting to make it perfect.  Far from it.  I'm just trying
to make it work for the use-cases that keep my scripts from working
properly or prevent me from doing something I'm trying to do in the
script..

> You may have a look over the fence at what Barebox fixed in this area,
> but the result will probably be the same.

I didn't even know that project existed.  Given how early they forked,
I'd be surprised if there is much to gain there, but there hasn't been
much in the way of changes to hush in either u-boot or barebox.  I'll
glace at it and see if there's anything obviously useful.

> I fear that such patches, while fixing a nasty problem here and there,
> will create other and maybe even more nasty problems in unexpacted
> areas.

I share your fear.  I try to find the simplest and most unobtrusive
ways to get the behavior I want in the hopes that these surgical
changes don't have unintended effects.

> My suggestion is to save such efforts, and rather spend the time on
> adating a more recent version of hush to U-Boot; getting things like
> shell functions or backtic evaluation would be more than welcome to
> many of us, I guess.

I agree it would be nice to have more shell features, but I fear it's
an awful lot of work.  At least for now I'm just trying to get key
functionality that I need, which can be fixed with minimal source
changes, and that I think others will find useful.

Where is the source project that hush came from anyway?  Some old
busybox?  Would you still recommend it over a more currently used
busybox shell?

-Joe

Patch

diff --git a/common/hush.c b/common/hush.c
index 4c84c2f..43edcfa 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -214,6 +214,7 @@  struct p_context {
 	int old_flag;				/* for figuring out valid reserved words */
 	struct p_context *stack;
 	int type;			/* define type of parser : ";$" common or special symbol */
+	pipe_style last_followup; /* PIPE_BG, PIPE_SEQ, PIPE_OR, PIPE_AND */
 	/* How about quoting status? */
 };
 
@@ -2534,7 +2535,7 @@  static int done_command(struct p_context *ctx)
 										) {
 #endif
 		debug_printf("done_command: skipping null command\n");
-		return 0;
+		return 1;
 	} else if (prog) {
 		pi->num_progs++;
 		debug_printf("done_command: num_progs incremented to %d\n",pi->num_progs);
@@ -2567,9 +2568,15 @@  static int done_command(struct p_context *ctx)
 static int done_pipe(struct p_context *ctx, pipe_style type)
 {
 	struct pipe *new_p;
-	done_command(ctx);  /* implicit closure of previous command */
+	int ret;
+
+	ret = done_command(ctx);  /* implicit closure of previous command */
+	/* The current command is null so don't allocate a new one */
+	if (ret && type == PIPE_SEQ)
+		return ret;
 	debug_printf("done_pipe, type %d\n", type);
 	ctx->pipe->followup = type;
+	ctx->last_followup = type;
 	ctx->pipe->r_mode = ctx->w;
 	new_p=new_pipe();
 	ctx->pipe->next = new_p;
@@ -2962,7 +2969,10 @@  int parse_stream(o_string *dest, struct p_context *ctx,
 				if (end_trigger != '\0' && ch=='\n')
 					done_pipe(ctx,PIPE_SEQ);
 			}
-			if (ch == end_trigger && !dest->quote && ctx->w==RES_NONE) {
+			if (ch == end_trigger && !dest->quote &&
+			    ctx->w == RES_NONE &&
+			    ctx->last_followup != PIPE_AND &&
+			    ctx->last_followup != PIPE_OR) {
 				debug_printf("leaving parse_stream (triggered)\n");
 				return 0;
 			}