Re: Test failure with negative substring offsets

[Peter Stephenson <Peter.Stephenson@xxxxxxx>]

On Mon, 23 May 2011 17:41:50 +0200
İsmail Dönmez <ismail@xxxxxxxxxxx> wrote:
> It passes for me on 64bit machines and fails on 32bit machines. Hope
> that helps.

ahhhhh... yes, that was exactly the clue I needed, thank you.

stdarg doesn't know the arguments need to be converted to integers to
fit the size being claimed in the printf prototype.

There might be a few more of these around, hidden by the fact that
there's only one integer argument and it's being passed little-endian,
so it would fail e.g. on Solaris 32-bit with large file support.

The second part is paranoia...  the stdarg manual says the va_list ap is
undefined after return from a function, so it's safest to run va_end()
on it within the function where it's been used.

Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.123
diff -p -u -r1.123 subst.c
--- Src/subst.c	19 May 2011 16:24:38 -0000	1.123
+++ Src/subst.c	23 May 2011 15:53:08 -0000
@@ -2892,7 +2892,7 @@ paramsubst(LinkList l, LinkNode n, char 
 			    length += alen - offset;
 			if (length < 0) {
 			    zerr("substring expression: %d < %d",
-			         length + offset, offset);
+			         (int)(length + offset), (int)offset);
 			    return NULL;
 			}
 		    } else
@@ -2942,7 +2942,8 @@ paramsubst(LinkList l, LinkNode n, char 
 			    }
 			    if (length < 0) {
 				zerr("substring expression: %d < %d",
-				     length + given_offset, given_offset);
+				     (int)(length + given_offset),
+				     (int)given_offset);
 				return NULL;
 			    }
 			}
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.258
diff -p -u -r1.258 utils.c
--- Src/utils.c	9 May 2011 09:49:09 -0000	1.258
+++ Src/utils.c	23 May 2011 15:53:09 -0000
@@ -155,7 +155,6 @@ VA_DCL
     VA_START(ap, fmt);
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(NULL, fmt, ap);
-    va_end(ap);
     errflag = 1;
 }
 
@@ -175,7 +174,6 @@ VA_DCL
     VA_GET_ARG(ap, cmd, const char *);
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(cmd, fmt, ap);
-    va_end(ap);
     errflag = 1;
 }
 
@@ -193,7 +191,6 @@ VA_DCL
     VA_START(ap, fmt);
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(NULL, fmt, ap);
-    va_end(ap);
 }
 
 /**/
@@ -212,7 +209,6 @@ VA_DCL
     VA_GET_ARG(ap, cmd, const char *);
     VA_GET_ARG(ap, fmt, const char *);
     zwarning(cmd, fmt, ap);
-    va_end(ap);
 }
 
 
@@ -236,7 +232,6 @@ VA_DCL
 	fclose(file);
     } else
 	zerrmsg(stderr, message, ap);
-    va_end(ap);
 }
 
 #endif /* DEBUG */
@@ -341,6 +336,8 @@ zerrmsg(FILE *file, const char *fmt, va_
 	}
     putc('\n', file);
     fflush(file);
+
+    va_end(ap);
 }
 
 /* Output a single character, for the termcap routines.     *

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


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom