Memory usage in read

[Peter Stephenson <pws@xxxxxxx>]

Playing with valgrind using my start-up files revealed the following.
The first hunk prevents one variable masking another (it's just cosmetic
but good practice); the second copies existing code from below (the
recent patch so read doesn't skip incomplete bits of multibyte
characters at the end of input); the final one addresses the error.
I don't think this error would actually have caused grief in real life,
since the variable set by the dodgy "if" would always have been set by
the code I've now moved into the chain of tests.

==23610== Conditional jump or move depends on uninitialised value(s)
==23610==    at 0x805FC05: bin_read (builtin.c:5467)
==23610==    by 0x805BC56: execbuiltin (builtin.c:439)
==23610==    by 0x80695FA: execcmd (exec.c:3067)
==23610==    by 0x806A717: execpline2 (exec.c:1561)
==23610==    by 0x806AAF6: execpline (exec.c:1347)
==23610==    by 0x806BA6D: execlist (exec.c:1144)
==23610==    by 0x8087B46: execwhile (loop.c:398)
==23610==    by 0x806912F: execcmd (exec.c:3008)
==23610==    by 0x806A717: execpline2 (exec.c:1561)
==23610==    by 0x806AAF6: execpline (exec.c:1347)
==23610==    by 0x806BA6D: execlist (exec.c:1144)
==23610==    by 0x8087883: execif (loop.c:515)

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.225
diff -u -r1.225 builtin.c
--- Src/builtin.c	7 May 2009 16:04:34 -0000	1.225
+++ Src/builtin.c	12 May 2009 10:58:41 -0000
@@ -5085,16 +5085,16 @@
     if (OPT_ISSET(ops,'d')) {
 	char *delimstr = OPT_ARG(ops,'d');
 #ifdef MULTIBYTE_SUPPORT
-	wint_t wc;
+	wint_t wi;
 
 	if (isset(MULTIBYTE)) {
 	    mb_metacharinit();
-	    (void)mb_metacharlenconv(delimstr, &wc);
+	    (void)mb_metacharlenconv(delimstr, &wi);
 	}
 	else
-	    wc = WEOF;
-	if (wc != WEOF)
-	    delim = (wchar_t)wc;
+	    wi = WEOF;
+	if (wi != WEOF)
+	    delim = (wchar_t)wi;
 	else
 	    delim = (wchar_t)((delimstr[0] == Meta) ?
 			      delimstr[1] ^ 32 : delimstr[0]);
@@ -5358,8 +5358,12 @@
 		wc = (wchar_t)c;
 	    }
 	    if (ret != MB_INCOMPLETE) {
-		if (ret == MB_INVALID)
+		if (ret == MB_INVALID) {
 		    memset(&mbs, 0, sizeof(mbs));
+		    /* Treat this as a single character */
+		    wc = (wchar_t)c;
+		    laststart = bptr;
+		}
 		if (bslash && wc == delim) {
 		    bslash = 0;
 		    continue;
@@ -5450,9 +5454,10 @@
 	}
 	signal_setmask(s);
 #ifdef MULTIBYTE_SUPPORT
-	if (c == EOF)
+	if (c == EOF) {
 	    gotnl = 1;
-	if (ret == MB_INCOMPLETE) {
+	    *bptr = '\0';	/* see below */
+	} else if (ret == MB_INCOMPLETE) {
 	    /*
 	     * We can only get here if there is an EOF in the
 	     * middle of a character... safest to keep the debris,


-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070