Bug 485

Created attachment 495 [details]
Correct diagnostic output

# make
  CC      shell/ash.o
cc1: warnings being treated as errors
shell/ash.c:10163: error: 'isneof' defined but not used
make[1]: *** [shell/ash.o] Error 1
make: *** [shell] Error 2

                for (lp = n->npipe.cmdlist; lp; lp = lp->next) {
-                       shcmd(lp->n, fp);
+                       shtree(lp->n, 0, NULL, fp);

Can you give an example when this change improves output?
Or is it fixes a bug?


This change does not actually change anything, AFAIKS:

-               n = n->nif.ifpart;
-               if (n->nif.elsepart) {
-                       cmdtxt(n);
+               if (!n->nif.elsepart) {
+                       n = n->nif.ifpart;
+               } else {
+                       cmdtxt(n->nif.ifpart);
                        cmdputs("; else ");
                        n = n->nif.elsepart;
                }

Why do you need it?

Created attachment 497 [details]
Patch which was applied to git. Checks for NEOF.

Basically, it adds

+       if (n == NODE_EOF) {
+               fputs("<EOF>", fp);
+               return;
+       }

to shtree(). The rest is just renaming of NEOF into NODE_EOF.

Other parts of your fix wait for your comments, Earl. :)

(In reply to comment #2)
> # make
>   CC      shell/ash.o
> cc1: warnings being treated as errors
> shell/ash.c:10163: error: 'isneof' defined but not used
> make[1]: *** [shell/ash.o] Error 1
> make: *** [shell] Error 2
> 

Oops ... need to wrap in DEBUG:

#ifdef DEBUG
static int isneof(union node *n)
{
        return n == NEOF;
}
#endif

(In reply to comment #3)

> This change does not actually change anything, AFAIKS:
> 
> -               n = n->nif.ifpart;
> -               if (n->nif.elsepart) {

I'm fairly certain this is a defect.

I think the code needs to consider n->nif.ifpart and n->nif.elsepart.

The code as written considers n->nif.ifpart and n->nif.ifpart->nif.elsepart.

(In reply to comment #3)
>                 for (lp = n->npipe.cmdlist; lp; lp = lp->next) {
> -                       shcmd(lp->n, fp);
> +                       shtree(lp->n, 0, NULL, fp);
> 
> Can you give an example when this change improves output?
> Or is it fixes a bug?

Late night.  I believe the following code breaks the original:


   ls | { while read ; do echo $f ; done ; }

(In reply to comment #6)
> (In reply to comment #3)
> > This change does not actually change anything, AFAIKS:
> > -               n = n->nif.ifpart;
> > -               if (n->nif.elsepart) {
> 
> I'm fairly certain this is a defect.
> 
> I think the code needs to consider n->nif.ifpart and n->nif.elsepart.
> 
> The code as written considers n->nif.ifpart and n->nif.ifpart->nif.elsepart.

*blush*

I see now. fixing...

(In reply to comment #7)
> >                 for (lp = n->npipe.cmdlist; lp; lp = lp->next) {
> > -                       shcmd(lp->n, fp);
> > +                       shtree(lp->n, 0, NULL, fp);
> > 
> > Can you give an example when this change improves output?
> > Or is it fixes a bug?
> 
> Late night.  I believe the following code breaks the original:
> 
>    ls | { while read ; do echo $f ; done ; }

Can't reproduce. But I tend to believe you. Applying it too.

Thanks!

Closing as fixed. If you can, please try current git and reopen if there are problems.