PATCH: and query for malloc(0).

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

Playing with POSIX regexes, I found some errors coming from zsh's memory
allocation with debugging on.  These turned out to resolve to some
malloc()s and free()s for zero length and the following oddity in
malloc():

    /* some systems want malloc to return the highest valid address plus one
       if it is called with an argument of zero */

    if (!size)
	return (MALLOC_RET_T) m_high;

Wuh...err...wuh...err...?   As I've commented in the patch below,

       TODO: really?  Suppose we allocate more memory, so
       that this is now in bounds, then a more rational application
       that thinks it can free() anything it malloc'ed, even
       of zero length, calls free for it?  Aren't we in big
       trouble?  Wouldn't it be safer just to allocate some
       memory anyway?

Actually, what was causing the immediate problem was a free() on
the address when it *was* still m_high, which was (correctly) being
flagged as an error by memory debugging (valgrind agreed something
was rotten).  But actually I think that's a side effect of the odd
code above.

This code already seemed to be ancient at the first mention of mem.c in
the mailing list archive in 1995.  If anyone knows anything, please
speak.  Otherwise I will commit the following patch.  valgrind and
the shell's own routines are much happier with it.

Index: Src/mem.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/mem.c,v
retrieving revision 1.14
diff -u -r1.14 mem.c
--- Src/mem.c	30 May 2006 22:35:03 -0000	1.14
+++ Src/mem.c	27 Apr 2007 21:24:56 -0000
@@ -830,10 +830,26 @@
 #endif
 
     /* some systems want malloc to return the highest valid address plus one
-       if it is called with an argument of zero */
+       if it is called with an argument of zero.
+    
+       TODO: really?  Suppose we allocate more memory, so
+       that this is now in bounds, then a more rational application
+       that thinks it can free() anything it malloc'ed, even
+       of zero length, calls free for it?  Aren't we in big
+       trouble?  Wouldn't it be safer just to allocate some
+       memory anyway?
+
+       If the above comment is really correct, then at least
+       we need to check in free() if we're freeing memory
+       at m_high.
+    */
 
     if (!size)
+#if 1
+	size = 1;
+#else
 	return (MALLOC_RET_T) m_high;
+#endif
 
     queue_signals();  /* just queue signals rather than handling them */
 
-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/