From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-32569-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 8053 invoked by alias); 20 Apr 2014 18:00:43 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 32569
Received: (qmail 5113 invoked from network); 20 Apr 2014 18:00:36 -0000
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au
X-Spam-Level: 
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE
	autolearn=ham version=3.3.2
From: Bart Schaefer <schaefer@brasslantern.com>
Message-id: <140420110020.ZM2511@torch.brasslantern.com>
Date: Sun, 20 Apr 2014 11:00:20 -0700
In-reply-to: <140419105445.ZM4656@torch.brasslantern.com>
Comments: In reply to Bart Schaefer <schaefer@brasslantern.com>
 "Re: SegFault in stringsubst" (Apr 19, 10:54am)
References: <20140416044612.GB24565@ewok>
	<140417114252.ZM22145@torch.brasslantern.com>	<20140418160344.GA10455@ewok>
	<140418100200.ZM23843@torch.brasslantern.com>
	<140419105445.ZM4656@torch.brasslantern.com>
X-Mailer: OpenZMail Classic (0.9.2 24April2005)
To: zsh-workers@zsh.org
Subject: Sanity of lastval ($?) in for/select loops
MIME-version: 1.0
Content-type: text/plain; charset=us-ascii

On Apr 19, 10:54am, Bart Schaefer wrote:
}
} ASIDE:
} 
} 	unset x
} 	for x in ${x:-$(exit 5)} y; do echo $?; done
} 
} outputs 5 in bash and 0 in zsh.  This suggests that perhaps execfor()
} should NOT be clobbering lastval.  (Same for "select".)

Data point: ksh93 agrees with bash here.

Furthermore, zsh execfor() and execselect() are inconsistent in their
handling of "lastval".  E.g., execfor() has this in the branch that
handles "for ((e1;e2;e3))" constructs:

        if (!errflag)
            matheval(str);
        if (errflag) {
            state->pc = end;
            return lastval = errflag;
        }

But later, in handling the loop body itself:

            if (errflag) {
                if (breaks)
                    breaks--;
                lastval = 1;
                break;
            }

So which should it be, lastval = 1 or lastval = errflag?  In practice I
don't think errflag is ever assigned anything other than 0 or 1 so it
probably isn't significant, just a bit confusing when reading the code.

However, execselect() then has:

    if (!args || empty(args)) {
	state->pc = end;
	return 1;
    }

Not "return lastval = 1" even though elsewhere it's careful to return
lastval.  Turns out the caller [execsimple() or execcmd()] always sets
lastval to whatever value is returned from execfor() or execselect()
or any of the other similar functions, so it's redundant to assign it
immediately before returning.

All of which presented as justification for this little patch:

diff --git a/Src/loop.c b/Src/loop.c
index dc8f232..2f639fd 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -73,7 +73,7 @@ execfor(Estate state, int do_exec)
 	    matheval(str);
 	if (errflag) {
 	    state->pc = end;
-	    return lastval = errflag;
+	    return 1;
 	}
 	cond = ecgetstr(state, EC_NODUP, &ctok);
 	advance = ecgetstr(state, EC_NODUP, &atok);
@@ -102,7 +102,7 @@ execfor(Estate state, int do_exec)
 		addlinknode(args, dupstring(*x));
 	}
     }
-    lastval = 0;
+    /* lastval = 0; */
     loops++;
     pushheap();
     cmdpush(CS_FOR);
@@ -241,7 +241,7 @@ execselect(Estate state, UNUSED(int do_exec))
 	return 1;
     }
     loops++;
-    lastval = 0;
+    /* lastval = 0; */
     pushheap();
     cmdpush(CS_SELECT);
     usezle = interact && SHTTY != -1 && isset(USEZLE);