[PATCH 0/4] dietlibc compatibility

[PATCH 0/4] dietlibc compatibility

[Eric Wong]

I've started statically-linking git binaries against dietlibc to avoid
having to recompile it for every machine/distro and chroot (lots!) I
would use it in.

For building git (on a Debian unstable system with dietlibc-dev),
I used the following make vars:

	CC=diet -v gcc
	NO_STRCASESTR=YesPlease
	NO_SETENV=YesPlease

The dietlibc setenv() doesn't seem very nice to **envp in git.c,
resulting in $PATH being clobbered when it runs execve().  This
caused tests to fail.  Fortunately, gitsetenv() saved the day.

The following patches fix other issues I had with dietlibc.

-- 
Eric Wong

[PATCH 1/4] git.c: extra #include for dietlibc (and possibly other C libraries)

[Eric Wong]

struct winsize is defined in <termios.h>, and that's not pulled
in by other #includes in that file

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 git.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

ae5641fcbc58509572d080c33a20c829b82ae9b0
diff --git a/git.c b/git.c
index e795ddb..434a3d9 100644
--- a/git.c
+++ b/git.c
@@ -9,6 +9,7 @@
 #include <limits.h>
 #include <stdarg.h>
 #include <sys/ioctl.h>
+#include <termios.h>
 #include "git-compat-util.h"
 
 #ifndef PATH_MAX
-- 
1.0.GIT

Re: [PATCH 1/4] git.c: extra #include for dietlibc (and possibly other C libraries)

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> struct winsize is defined in <termios.h>, and that's not pulled
> in by other #includes in that file

With glibc, sys/ioctl.h seems to pull it in.  Input from people
on other platforms is appreciated on this.

[PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[Eric Wong]

dietlibc versions of malloc, calloc and realloc all return NULL if
they're told to allocate 0 bytes, causes the x* wrappers to die().

There are several more places where these calls could end up asking
for 0 bytes, too...

Maybe simply not die()-ing in the x* wrappers if 0/NULL is returned
when the requested size is zero is a safer and easier way to go.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 commit.c          |    3 +++
 diffcore-rename.c |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

ee9d90c652be126345dec2ac204284e80e685160
diff --git a/commit.c b/commit.c
index e867b86..edd4ded 100644
--- a/commit.c
+++ b/commit.c
@@ -560,6 +560,9 @@ void sort_in_topological_order(struct co
 		next = next->next;
 		count++;
 	}
+	
+	if (!count)
+		return;
 	/* allocate an array to help sort the list */
 	nodes = xcalloc(count, sizeof(*nodes));
 	/* link the list to the array */
diff --git a/diffcore-rename.c b/diffcore-rename.c
index dba965c..39d9126 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -282,7 +282,7 @@ void diffcore_rename(struct diff_options
 		else if (detect_rename == DIFF_DETECT_COPY)
 			register_rename_src(p->one, 1);
 	}
-	if (rename_dst_nr == 0 ||
+	if (rename_dst_nr == 0 || rename_src_nr == 0 ||
 	    (0 < rename_limit && rename_limit < rename_dst_nr))
 		goto cleanup; /* nothing to do */
 
-- 
1.0.GIT

Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[Johannes Schindelin]

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> dietlibc versions of malloc, calloc and realloc all return NULL if
> they're told to allocate 0 bytes, causes the x* wrappers to die().
> 
> There are several more places where these calls could end up asking
> for 0 bytes, too...
> 
> Maybe simply not die()-ing in the x* wrappers if 0/NULL is returned
> when the requested size is zero is a safer and easier way to go.

I would prefer that, too. But the cleanest way would be to prevent calls 
to *alloc if the size is 0...

Ciao,
Dscho

Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[H. Peter Anvin]

Eric Wong wrote:
> dietlibc versions of malloc, calloc and realloc all return NULL if
> they're told to allocate 0 bytes, causes the x* wrappers to die().
> 
> There are several more places where these calls could end up asking
> for 0 bytes, too...
> 
> Maybe simply not die()-ing in the x* wrappers if 0/NULL is returned
> when the requested size is zero is a safer and easier way to go.
> 

Better yet, either always return NULL or allocate 1 byte in that case, 
to get consistent behaviour.

	-hpa

Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[Linus Torvalds]

On Tue, 27 Dec 2005, H. Peter Anvin wrote:
> 
> Better yet, either always return NULL or allocate 1 byte in that case, to get
> consistent behaviour.

Yes. However, if you do the "return NULL" case (which is nicest), you'll 
have to wrap "free()" too. There are some libraries where passing "free()" 
a NULL pointer causes a SIGSEGV.

That said, I think that would be preferable to changing the source code to 
unnecessarily avoid zero-sized allocations. Having a "xfree()" to match 
"xmalloc()" makes sense. 

		Linus

Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> That said, I think that would be preferable to changing the source code to 
> unnecessarily avoid zero-sized allocations.

Yes, that has essentially been the plan (according to the
discussion lead to 7e4a2a848377241b8fb4f624d1151bbf2f8d5814
commit on the list).

After eradicating zero-sized allocations where that change makes
the overall code cleaner (which Johannes and Eric did most of
the heavylifting and I think mostly done), we would apply
something like this, instead of doing x*alloc(size ? size : 1)
at the calling site.

About die(), I think the current code structure is fine.  If we
were doing a library, propagating NULL from C library *alloc()
back to our caller and having the caller deal with it is the
right thing, but most of the callers of x*alloc() are our main
programs and there aren't much they can do when we run out of
memory.

---
diff --git a/git-compat-util.h b/git-compat-util.h
index 0c98c99..a71728e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -63,6 +63,8 @@ extern char *gitstrcasestr(const char *h
 static inline void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
+	if (!ret && !size)
+		ret = malloc(1); /* funny c library */
 	if (!ret)
 		die("Out of memory, malloc failed");
 	return ret;
@@ -71,6 +73,8 @@ static inline void *xmalloc(size_t size)
 static inline void *xrealloc(void *ptr, size_t size)
 {
 	void *ret = realloc(ptr, size);
+	if (!ret && !size)
+		ret = realloc(ptr, 1); /* funny c library */
 	if (!ret)
 		die("Out of memory, realloc failed");
 	return ret;
@@ -79,6 +83,8 @@ static inline void *xrealloc(void *ptr, 
 static inline void *xcalloc(size_t nmemb, size_t size)
 {
 	void *ret = calloc(nmemb, size);
+	if (!ret && (!nmemb || !size))
+		ret = calloc(1, 1); /* funny c library */
 	if (!ret)
 		die("Out of memory, calloc failed");
 	return ret;

Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes

[H. Peter Anvin]

Linus Torvalds wrote:
> 
> On Tue, 27 Dec 2005, H. Peter Anvin wrote:
> 
>>Better yet, either always return NULL or allocate 1 byte in that case, to get
>>consistent behaviour.
> 
> Yes. However, if you do the "return NULL" case (which is nicest), you'll 
> have to wrap "free()" too. There are some libraries where passing "free()" 
> a NULL pointer causes a SIGSEGV.
> 
> That said, I think that would be preferable to changing the source code to 
> unnecessarily avoid zero-sized allocations. Having a "xfree()" to match 
> "xmalloc()" makes sense. 
> 

Yeah, although that might break GNU code which uses xmalloc that is 
included (GNU doesn't have xfree.)  The easiest is just to allocate 1 
byte when the user asks for 0.

Anyone knows what GNU xmalloc does?

	-hpa

[PATCH 3/4] add xmktime() function that always accounts for the TZ env

[Eric Wong]

This function was added because mktime in dietlibc doesn't seem to
account for the TZ env.  Also, xmktime() now shares the same
always-summer bug TZ parsing elsewhere, so at least we can
be wrong about summer consistently.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 cache.h           |    2 ++
 convert-objects.c |    4 ++--
 date.c            |   30 ++++++++++++++++++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)

3b0763ae6fce6c69021e1216660f4c0ee301512b
diff --git a/cache.h b/cache.h
index cb87bec..c5ff4b7 100644
--- a/cache.h
+++ b/cache.h
@@ -5,6 +5,7 @@
 
 #include SHA1_HEADER
 #include <zlib.h>
+#include <time.h>
 
 #if ZLIB_VERNUM < 0x1200
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
@@ -236,6 +237,7 @@ extern void *read_object_with_reference(
 
 const char *show_date(unsigned long time, int timezone);
 int parse_date(const char *date, char *buf, int bufsize);
+time_t xmktime(struct tm *tm);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
 
diff --git a/convert-objects.c b/convert-objects.c
index b49bce2..0fe1229 100644
--- a/convert-objects.c
+++ b/convert-objects.c
@@ -178,7 +178,7 @@ static unsigned long parse_oldstyle_date
 		const char *next = strptime(buf, *fmt, &tm);
 		if (next) {
 			if (!*next)
-				return mktime(&tm);
+				return xmktime(&tm);
 			buf = next;
 		} else {
 			const char **p = timezones;
@@ -195,7 +195,7 @@ static unsigned long parse_oldstyle_date
 		fmt++;
 	} while (*buf && *fmt);
 	printf("left: %s\n", buf);
-	return mktime(&tm);				
+	return xmktime(&tm);				
 }
 
 static int convert_date_line(char *dst, void **buf, unsigned long *sp)
diff --git a/date.c b/date.c
index 3e11500..5596476 100644
--- a/date.c
+++ b/date.c
@@ -141,6 +141,28 @@ static int match_string(const char *date
 	return i;
 }
 
+time_t xmktime(struct tm *tm)
+{
+	time_t ret = my_mktime(tm);
+	char * tz = getenv("TZ");
+
+	if (tz) {
+		int i;
+		for (i = 0; i < NR_TZ; i++) {
+			int match = match_string(tz, timezone_names[i].name);
+			if (match >= 3) {
+				int off = timezone_names[i].offset;
+
+				/* This is bogus, but we like summer */
+				off += timezone_names[i].dst;
+				
+				ret += 60*off;
+			}
+		}
+	}
+	return ret;
+}
+
 static int skip_alpha(const char *date)
 {
 	int i = 0;
@@ -436,10 +458,10 @@ int parse_date(const char *date, char *r
 		date += match;
 	}
 
-	/* mktime uses local timezone */
+	/* (x)mktime uses local timezone */
 	then = my_mktime(&tm); 
 	if (offset == -1)
-		offset = (then - mktime(&tm)) / 60;
+		offset = (then - xmktime(&tm)) / 60;
 
 	if (then == -1)
 		return -1;
@@ -464,7 +486,7 @@ void datestamp(char *buf, int bufsize)
 
 static void update_tm(struct tm *tm, unsigned long sec)
 {
-	time_t n = mktime(tm) - sec;
+	time_t n = xmktime(tm) - sec;
 	localtime_r(&n, tm);
 }
 
@@ -642,5 +664,5 @@ unsigned long approxidate(const char *da
 		tm.tm_mday = number;
 	if (tm.tm_mon > now.tm_mon)
 		tm.tm_year--;
-	return mktime(&tm);
+	return xmktime(&tm);
 }
-- 
1.0.GIT

Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env

[Johannes Schindelin]

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> This function was added because mktime in dietlibc doesn't seem to
> account for the TZ env.  Also, xmktime() now shares the same
> always-summer bug TZ parsing elsewhere, so at least we can
> be wrong about summer consistently.

How about making this a compat function, which is used when the Makefile 
variable MKTIME_IGNORES_TZ is set?

Ciao,
Dscho

Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> This function was added because mktime in dietlibc doesn't seem to
> account for the TZ env.  Also, xmktime() now shares the same
> always-summer bug TZ parsing elsewhere,

Where elsewhere?

Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > This function was added because mktime in dietlibc doesn't seem to
> > account for the TZ env.  Also, xmktime() now shares the same
> > always-summer bug TZ parsing elsewhere,
> 
> Where elsewhere?
 
match_alpha() in date.c

-- 
Eric Wong

Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Eric Wong <normalperson@yhbt.net> writes:
>> 
>> > This function was added because mktime in dietlibc doesn't seem to
>> > account for the TZ env.  Also, xmktime() now shares the same
>> > always-summer bug TZ parsing elsewhere,
>> 
>> Where elsewhere?
>  
> match_alpha() in date.c

Then probably we should extract partime.c from rcs and munge
that.

[PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Eric Wong]

dietlibc versions of these allocators returns NULL if a size of zero is
specified.  Obviously, this is a problem with the x* wrappers since
we check for them returning NULL.

Down the line, it'd be better to hunt down and eliminate all calls to
these functions when they are called with a zero argument.  I've already
added some checks for these cases that were exposed by tests.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 git-compat-util.h |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

113dca27f9f95a76a0f98929720f5f567c7586b2
diff --git a/git-compat-util.h b/git-compat-util.h
index 0c98c99..bd2f150 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -62,7 +62,12 @@ extern char *gitstrcasestr(const char *h
 
 static inline void *xmalloc(size_t size)
 {
-	void *ret = malloc(size);
+	void *ret;
+	
+	if (!size)
+		return NULL;
+	
+	ret = malloc(size);
 	if (!ret)
 		die("Out of memory, malloc failed");
 	return ret;
@@ -70,7 +75,14 @@ static inline void *xmalloc(size_t size)
 
 static inline void *xrealloc(void *ptr, size_t size)
 {
-	void *ret = realloc(ptr, size);
+	void *ret;
+	
+	if (!size) {
+		free(ptr);
+		return NULL;
+	}
+	
+	ret = realloc(ptr, size);
 	if (!ret)
 		die("Out of memory, realloc failed");
 	return ret;
@@ -78,7 +90,12 @@ static inline void *xrealloc(void *ptr, 
 
 static inline void *xcalloc(size_t nmemb, size_t size)
 {
-	void *ret = calloc(nmemb, size);
+	void *ret;
+	
+	if (!nmemb || !size)
+		return NULL;
+	
+	ret = calloc(nmemb, size);
 	if (!ret)
 		die("Out of memory, calloc failed");
 	return ret;
-- 
1.0.GIT

Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> dietlibc versions of these allocators returns NULL if a size of zero is
> specified.  Obviously, this is a problem with the x* wrappers since
> we check for them returning NULL.
>
> Down the line, it'd be better to hunt down and eliminate all calls to
> these functions when they are called with a zero argument.  I've already
> added some checks for these cases that were exposed by tests.

I agree that it is a bug to rely on *alloc(0) not returning
NULL, but this patch is too risky.  It would be a good thing to
have debugging variant of x* wrappers that barf on a 0-byte
allocation request to find the offending callers, instead of
returning NULL, maybe like the attached patch.

Since eradicating *alloc(0) calls is the right way to go, but it
takes time.  Touching x* wrappers for general public should not
be done before it is done.  It breaks things for everybody,
while the current code is broken only for diet people and
developers that use the debugging variant of x* wrappers.

-- >8 --
diff --git a/git-compat-util.h b/git-compat-util.h
index 0c98c99..08fd6bc 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -60,9 +60,17 @@ extern int gitsetenv(const char *, const
 extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
+#ifdef DEBUG_0ALLOC
+#define debug_0alloc(sz) assert(0 < (sz))
+#else
+#define debug_0alloc(sz)
+#endif
+
 static inline void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
+
+	debug_0alloc(size);
 	if (!ret)
 		die("Out of memory, malloc failed");
 	return ret;
@@ -71,6 +79,7 @@ static inline void *xmalloc(size_t size)
 static inline void *xrealloc(void *ptr, size_t size)
 {
 	void *ret = realloc(ptr, size);
+	debug_0alloc(size);
 	if (!ret)
 		die("Out of memory, realloc failed");
 	return ret;
@@ -79,6 +88,7 @@ static inline void *xrealloc(void *ptr, 
 static inline void *xcalloc(size_t nmemb, size_t size)
 {
 	void *ret = calloc(nmemb, size);
+	debug_0alloc(nmemb);
 	if (!ret)
 		die("Out of memory, calloc failed");
 	return ret;

Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > dietlibc versions of these allocators returns NULL if a size of zero is
> > specified.  Obviously, this is a problem with the x* wrappers since
> > we check for them returning NULL.
> >
> > Down the line, it'd be better to hunt down and eliminate all calls to
> > these functions when they are called with a zero argument.  I've already
> > added some checks for these cases that were exposed by tests.
> 
> I agree that it is a bug to rely on *alloc(0) not returning
> NULL, but this patch is too risky.  It would be a good thing to
> have debugging variant of x* wrappers that barf on a 0-byte
> allocation request to find the offending callers, instead of
> returning NULL, maybe like the attached patch.
> 
> Since eradicating *alloc(0) calls is the right way to go, but it
> takes time.  Touching x* wrappers for general public should not
> be done before it is done.  It breaks things for everybody,
> while the current code is broken only for diet people and
> developers that use the debugging variant of x* wrappers.

I'll be using the following patch to collect results for the next few
days without interrupting my work on other projects.  I'll post
patches to remove *alloc(0) calls when I find them.

diff --git a/git-compat-util.h b/git-compat-util.h
index 0c98c99..43be289 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -60,9 +60,41 @@ extern int gitsetenv(const char *, const
 extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
+static void fork_for_core (const char * caller)
+{
+	pid_t child = fork();
+	if (child == 0) {
+		int i;
+		char out[4096];
+		char * progname = "unknown";
+		FILE * dbg = fopen("/var/tmp/git_alloc_dbg.log","a");
+
+		for(i = 0; environ[i]; i++) {
+			if (strstr(environ[i],"_=") == environ[i])
+				progname = environ[i];
+		}
+		
+		snprintf(out, 4096, "%s:%s: pid %d dumping core for pid %d\n",
+				progname, caller, getpid(), getppid());
+
+		fputs(out, stderr);
+		fputs(out, dbg);
+		fflush(NULL);
+		
+		abort();
+	}
+}
+
 static inline void *xmalloc(size_t size)
 {
-	void *ret = malloc(size);
+	void *ret;
+	
+	if (!size) {
+		fork_for_core(__func__);
+		return NULL;
+	}
+	
+	ret = malloc(size);
 	if (!ret)
 		die("Out of memory, malloc failed");
 	return ret;
@@ -70,7 +102,15 @@ static inline void *xmalloc(size_t size)
 
 static inline void *xrealloc(void *ptr, size_t size)
 {
-	void *ret = realloc(ptr, size);
+	void *ret;
+	
+	if (!size) {
+		fork_for_core(__func__);
+		free(ptr);
+		return NULL;
+	}
+	
+	ret = realloc(ptr, size);
 	if (!ret)
 		die("Out of memory, realloc failed");
 	return ret;
@@ -78,7 +118,14 @@ static inline void *xrealloc(void *ptr, 
 
 static inline void *xcalloc(size_t nmemb, size_t size)
 {
-	void *ret = calloc(nmemb, size);
+	void *ret;
+	
+	if (!nmemb || !size) {
+		fork_for_core(__func__);
+		return NULL;
+	}
+
+	ret = calloc(nmemb, size);
 	if (!ret)
 		die("Out of memory, calloc failed");
 	return ret;

-- 
Eric Wong

[PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Johannes Schindelin]

This is the result of a relatively quick audit of the source code. There
might still be a few odd places lurking out there, but I am quite certain
I caught most if not all.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 csum-file.c         |    3 +++
 diff.c              |    6 +++--
 diffcore-order.c    |    8 ++++++-
 diffcore-pathspec.c |   12 +++++++----
 index-pack.c        |   11 ++++++----
 pack-objects.c      |    6 ++++-
 read-tree.c         |   17 +++++++++------
 sha1_file.c         |   56 ++++++++++++++++++++++++++-------------------------
 tree-diff.c         |    4 ++++
 unpack-objects.c    |    2 +-
 10 files changed, 76 insertions(+), 49 deletions(-)

751737aeec4dd3cc1afd1d1d3fa529cfc74535e0
diff --git a/csum-file.c b/csum-file.c
index 5f9249a..2c0f097 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
 	unsigned long maxsize;
 	void *out;
 
+	if (size == 0)
+		return 0;
+
 	memset(&stream, 0, sizeof(stream));
 	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
 	maxsize = deflateBound(&stream, size);
diff --git a/diff.c b/diff.c
index c815918..daee05b 100644
--- a/diff.c
+++ b/diff.c
@@ -504,9 +504,9 @@ static void prepare_temp_file(const char
 		}
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
-			char *buf, buf_[1024];
-			buf = ((sizeof(buf_) < st.st_size) ?
-			       xmalloc(st.st_size) : buf_);
+			char buf[MAXPATHLEN + 1];
+			if (st.st_size > MAXPATHLEN)
+				die("Unbelievably long symlink: %", name);
 			ret = readlink(name, buf, st.st_size);
 			if (ret < 0)
 				die("readlink(%s)", name);
diff --git a/diffcore-order.c b/diffcore-order.c
index b381223..94a4996 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -61,6 +61,8 @@ static void prepare_order(const char *or
 		}
 		if (pass == 0) {
 			order_cnt = cnt;
+			if (cnt == 0)
+				return;
 			order = xmalloc(sizeof(*order) * cnt);
 		}
 	}
@@ -105,9 +107,13 @@ static int compare_pair_order(const void
 void diffcore_order(const char *orderfile)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	struct pair_order *o;
 	int i;
 
+	if (q->nr == 0)
+		return;
+
+	o = xmalloc(sizeof(*o) * q->nr);
 	prepare_order(orderfile);
 	for (i = 0; i < q->nr; i++) {
 		o[i].pair = q->queue[i];
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
index 68fe009..a12337a 100644
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path
 	for (i = 0; pathspec[i]; i++)
 		;
 	speccnt = i;
-	spec = xmalloc(sizeof(*spec) * speccnt);
-	for (i = 0; pathspec[i]; i++) {
-		spec[i].spec = pathspec[i];
-		spec[i].len = strlen(pathspec[i]);
+	if (speccnt == 0)
+		spec = NULL;
+	else {
+		spec = xmalloc(sizeof(*spec) * speccnt);
+		for (i = 0; pathspec[i]; i++) {
+			spec[i].spec = pathspec[i];
+			spec[i].len = strlen(pathspec[i]);
+		}
 	}
 
 	for (i = 0; i < q->nr; i++) {
diff --git a/index-pack.c b/index-pack.c
index d4ce3af..2927632 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -103,7 +103,7 @@ static void *unpack_entry_data(unsigned 
 	unsigned long pack_limit = pack_size - 20;
 	unsigned long pos = *current_pos;
 	z_stream stream;
-	void *buf = xmalloc(size);
+	void *buf = xmalloc(size ? size : 1);
 
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = buf;
@@ -353,7 +353,8 @@ static void write_index_file(const char 
 {
 	struct sha1file *f;
 	struct object_entry **sorted_by_sha =
-		xcalloc(nr_objects, sizeof(struct object_entry *));
+		xcalloc(nr_objects ? nr_objects : 1,
+				sizeof(struct object_entry *));
 	struct object_entry **list = sorted_by_sha;
 	struct object_entry **last = sorted_by_sha + nr_objects;
 	unsigned int array[256];
@@ -448,8 +449,10 @@ int main(int argc, char **argv)
 
 	open_pack_file();
 	parse_pack_header();
-	objects = xcalloc(nr_objects, sizeof(struct object_entry));
-	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
+	objects = xcalloc(nr_objects ? nr_objects : 1,
+			sizeof(struct object_entry));
+	deltas = xcalloc(nr_objects ? nr_objects : 1,
+			sizeof(struct delta_entry));
 	parse_pack_objects();
 	free(deltas);
 	write_index_file(index_name, sha1);
diff --git a/pack-objects.c b/pack-objects.c
index caf3b6b..f5a147f 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -406,7 +406,8 @@ static void prepare_pack(int window, int
 
 	fprintf(stderr, "Packing %d objects\n", nr_objects);
 
-	sorted_by_type = create_sorted_list(type_size_sort);
+	if (nr_objects > 0)
+		sorted_by_type = create_sorted_list(type_size_sort);
 	if (window && depth)
 		find_deltas(sorted_by_type, window+1, depth);
 	write_pack_file();
@@ -540,7 +541,8 @@ int main(int argc, char **argv)
 	if (non_empty && !nr_objects)
 		return 0;
 
-	sorted_by_sha = create_sorted_list(sha1_sort);
+	if (nr_objects > 0)
+		sorted_by_sha = create_sorted_list(sha1_sort);
 	SHA1_Init(&ctx);
 	list = sorted_by_sha;
 	for (i = 0; i < nr_objects; i++) {
diff --git a/read-tree.c b/read-tree.c
index e3b9c0d..a46c6fe 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -294,17 +294,20 @@ static int unpack_trees(merge_fn_t fn)
 {
 	int indpos = 0;
 	unsigned len = object_list_length(trees);
-	struct tree_entry_list **posns = 
-		xmalloc(len * sizeof(struct tree_entry_list *));
+	struct tree_entry_list **posns;
 	int i;
 	struct object_list *posn = trees;
 	merge_size = len;
-	for (i = 0; i < len; i++) {
-		posns[i] = ((struct tree *) posn->item)->entries;
-		posn = posn->next;
+
+	if (len) {
+		posns = xmalloc(len * sizeof(struct tree_entry_list *));
+		for (i = 0; i < len; i++) {
+			posns[i] = ((struct tree *) posn->item)->entries;
+			posn = posn->next;
+		}
+		if (unpack_trees_rec(posns, len, "", fn, &indpos))
+			return -1;
 	}
-	if (unpack_trees_rec(posns, len, "", fn, &indpos))
-		return -1;
 
 	if (trivial_merges_only && nontrivial_merge)
 		die("Merge requires file-level merging");
diff --git a/sha1_file.c b/sha1_file.c
index 8bebbb2..0d60884 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1371,35 +1371,37 @@ int write_sha1_file(void *buf, unsigned 
 		return -1;
 	}
 
-	/* Set it up */
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, Z_BEST_COMPRESSION);
-	size = deflateBound(&stream, len+hdrlen);
-	compressed = xmalloc(size);
-
-	/* Compress it */
-	stream.next_out = compressed;
-	stream.avail_out = size;
-
-	/* First header.. */
-	stream.next_in = hdr;
-	stream.avail_in = hdrlen;
-	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */;
-
-	/* Then the data itself.. */
-	stream.next_in = buf;
-	stream.avail_in = len;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&stream);
-	size = stream.total_out;
+	if (len + hdrlen > 0) {
+		/* Set it up */
+		memset(&stream, 0, sizeof(stream));
+		deflateInit(&stream, Z_BEST_COMPRESSION);
+		size = deflateBound(&stream, len+hdrlen);
+		compressed = xmalloc(size);
+
+		/* Compress it */
+		stream.next_out = compressed;
+		stream.avail_out = size;
 
-	if (write(fd, compressed, size) != size)
-		die("unable to write file");
+		/* First header.. */
+		stream.next_in = hdr;
+		stream.avail_in = hdrlen;
+		while (deflate(&stream, 0) == Z_OK)
+			/* nothing */;
+
+		/* Then the data itself.. */
+		stream.next_in = buf;
+		stream.avail_in = len;
+		while (deflate(&stream, Z_FINISH) == Z_OK)
+			/* nothing */;
+		deflateEnd(&stream);
+		size = stream.total_out;
+
+		if (write(fd, compressed, size) != size)
+			die("unable to write file");
+		free(compressed);
+	}
 	fchmod(fd, 0444);
 	close(fd);
-	free(compressed);
 
 	return move_temp_to_file(tmpfile, filename);
 }
@@ -1429,7 +1431,7 @@ int write_sha1_to_fd(int fd, const unsig
 		memset(&stream, 0, sizeof(stream));
 		deflateInit(&stream, Z_BEST_COMPRESSION);
 		size = deflateBound(&stream, len + hdrlen);
-		temp_obj = buf = xmalloc(size);
+		temp_obj = buf = xmalloc(size ? size : 1);
 
 		/* Compress it */
 		stream.next_out = buf;
diff --git a/tree-diff.c b/tree-diff.c
index 0ef06a9..382092b 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -263,6 +263,10 @@ void diff_tree_setup_paths(const char **
 
 		paths = p;
 		nr_paths = count_paths(paths);
+		if (nr_paths == 0) {
+			pathlens = NULL;
+			return;
+		}
 		pathlens = xmalloc(nr_paths * sizeof(int));
 		for (i=0; i<nr_paths; i++)
 			pathlens[i] = strlen(paths[i]);
diff --git a/unpack-objects.c b/unpack-objects.c
index 5c5cb12..572eeee 100644
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -53,7 +53,7 @@ static void use(int bytes)
 static void *get_data(unsigned long size)
 {
 	z_stream stream;
-	void *buf = xmalloc(size);
+	void *buf = xmalloc(size ? size : 1);
 
 	memset(&stream, 0, sizeof(stream));
 
-- 
1.0.GIT

Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This is the result of a relatively quick audit of the source code. There
> might still be a few odd places lurking out there, but I am quite certain
> I caught most if not all.

Thanks.

> diff --git a/csum-file.c b/csum-file.c
> index 5f9249a..2c0f097 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
>  	unsigned long maxsize;
>  	void *out;
>  
> +	if (size == 0)
> +		return 0;
> +
>  	memset(&stream, 0, sizeof(stream));
>  	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
>  	maxsize = deflateBound(&stream, size);

I think this and the one in sha1_file.c::write_sha1_file() are
wrong; 0-size input would not result in 0-size output.  Have you
tested them by actually exercising the codepaths you touched?

> diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
> index 68fe009..a12337a 100644
> --- a/diffcore-pathspec.c
> +++ b/diffcore-pathspec.c
> @@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path

diffcore-pathspec and diffcore-order can probably return without
touching diff_queued_diff if there is no work to be done.

> @@ -353,7 +353,8 @@ static void write_index_file(const char 
>  {
>  	struct sha1file *f;
>  	struct object_entry **sorted_by_sha =
> -		xcalloc(nr_objects, sizeof(struct object_entry *));
> +		xcalloc(nr_objects ? nr_objects : 1,
> +				sizeof(struct object_entry *));
>  	struct object_entry **list = sorted_by_sha;
>  	struct object_entry **last = sorted_by_sha + nr_objects;
>  	unsigned int array[256];

This can be simplified by sorted_by_sha = list = last = NULL
when nr_objects == 0 and avoiding qsort; that is what you did in
pack-objects, I think.

> @@ -448,8 +449,10 @@ int main(int argc, char **argv)
>  
>  	open_pack_file();
>  	parse_pack_header();
> -	objects = xcalloc(nr_objects, sizeof(struct object_entry));
> -	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
> +	objects = xcalloc(nr_objects ? nr_objects : 1,
> +			sizeof(struct object_entry));
> +	deltas = xcalloc(nr_objects ? nr_objects : 1,
> +			sizeof(struct delta_entry));
>  	parse_pack_objects();
>  	free(deltas);
>  	write_index_file(index_name, sha1);

Likewise I suspect.  After all the special case is only when
reindexing an empty pack ;-).

My inclination is to do things in these steps:

 - apply cleanups that actually simplify the logic, while
   leaving the ones that you needed to do (size ?  size : 1)
   unmodified (BTW, must next_in/next_out point at non NULL when
   avail_in/avail_out are zero?).

 - change x*alloc like this, once the above is done:

        static inline void *xmalloc(size_t size)
        {
                void *ret = malloc(size);
        #ifdef MALLOC_CAN_RETURN_NULL_ON_0SIZE
                if (!ret && !size)
                        ret = malloc(size+1);
        #endif
                if (!ret)
                        die("Out of memory, malloc failed");
                return ret;
        }

Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Johannes Schindelin]

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/csum-file.c b/csum-file.c
> > index 5f9249a..2c0f097 100644
> > --- a/csum-file.c
> > +++ b/csum-file.c
> > @@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
> >  	unsigned long maxsize;
> >  	void *out;
> >  
> > +	if (size == 0)
> > +		return 0;
> > +
> >  	memset(&stream, 0, sizeof(stream));
> >  	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
> >  	maxsize = deflateBound(&stream, size);
> 
> I think this and the one in sha1_file.c::write_sha1_file() are
> wrong; 0-size input would not result in 0-size output.  Have you
> tested them by actually exercising the codepaths you touched?

No, I did not test them. At least not consciously (I did the whole work on 
Dec 24, when I was lying in bed, ill, trying to distract myself from being 
miserable).

The reason I did it this way: If zlib inflates a buffer of 0 bytes, it 
makes no sense to expect anything than 0 bytes to come out of it, right? 
Therefore, if zlib encounters a deflated buffer of 0 bytes, it should 
inflate it to 0 bytes. So it is a good idea in any case to write out 0 
bytes.

However, thinking of it again, it might break backwards compatibility. But 
I don't think so.

> > diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
> > index 68fe009..a12337a 100644
> > --- a/diffcore-pathspec.c
> > +++ b/diffcore-pathspec.c
> > @@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path
> 
> diffcore-pathspec and diffcore-order can probably return without
> touching diff_queued_diff if there is no work to be done.

To be honest: I did not completely understand what that code does. 
It seemed obvious, but I got a track record of breaking things. So I was 
cautious. I will not try to modify it myself without a good test case.

> > @@ -353,7 +353,8 @@ static void write_index_file(const char 
> >  {
> >  	struct sha1file *f;
> >  	struct object_entry **sorted_by_sha =
> > -		xcalloc(nr_objects, sizeof(struct object_entry *));
> > +		xcalloc(nr_objects ? nr_objects : 1,
> > +				sizeof(struct object_entry *));
> >  	struct object_entry **list = sorted_by_sha;
> >  	struct object_entry **last = sorted_by_sha + nr_objects;
> >  	unsigned int array[256];
> 
> This can be simplified by sorted_by_sha = list = last = NULL
> when nr_objects == 0 and avoiding qsort; that is what you did in
> pack-objects, I think.

If nr_objects == 0, we could write out a fixed index.

We could alternatively exit(1), because what sense does it make to have an 
index if the pack contains 0 objects? However, this has consequences: 
Every user of index-pack (does "fetch-pack --keep" call it?) has to be 
aware that empty packs are illegal.

Again, I did this so that the tool does not break. I'll try to come up 
with something better soon.

> > @@ -448,8 +449,10 @@ int main(int argc, char **argv)
> >  
> >  	open_pack_file();
> >  	parse_pack_header();
> > -	objects = xcalloc(nr_objects, sizeof(struct object_entry));
> > -	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
> > +	objects = xcalloc(nr_objects ? nr_objects : 1,
> > +			sizeof(struct object_entry));
> > +	deltas = xcalloc(nr_objects ? nr_objects : 1,
> > +			sizeof(struct delta_entry));
> >  	parse_pack_objects();
> >  	free(deltas);
> >  	write_index_file(index_name, sha1);
> 
> Likewise I suspect.  After all the special case is only when
> reindexing an empty pack ;-).

Right.

> My inclination is to do things in these steps:
> 
>  - apply cleanups that actually simplify the logic, while
>    leaving the ones that you needed to do (size ?  size : 1)
>    unmodified (BTW, must next_in/next_out point at non NULL when
>    avail_in/avail_out are zero?).

I was cautious. Maybe at some point in future, zlib relies on that. But it 
is probably better to try not calling zlib at all in these cases.

>  - change x*alloc like this, once the above is done:
> 
>         static inline void *xmalloc(size_t size)
>         {
>                 void *ret = malloc(size);
>         #ifdef MALLOC_CAN_RETURN_NULL_ON_0SIZE
>                 if (!ret && !size)
>                         ret = malloc(size+1);
>         #endif
>                 if (!ret)
>                         die("Out of memory, malloc failed");
>                 return ret;
>         }

Sounds easiest.

Ciao,
Dscho

[PATCH] avoid asking ?alloc() for zero bytes.

[Junio C Hamano]

Avoid asking for zero bytes when that change simplifies overall
logic.  Later we would change the wrapper to ask for 1 byte on
platforms that return NULL for zero byte request.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * Here is what I have now that roughly corresponds to your
   patch, which has been somewhat tested.

   The final phase of making sure we return something from
   x*alloc() is not done yet.

 diff.c              |    6 +++---
 diffcore-order.c    |    6 +++++-
 diffcore-pathspec.c |    3 +++
 index-pack.c        |   22 ++++++++++++++--------
 read-tree.c         |   17 ++++++++++-------
 tree-diff.c         |    4 ++++
 6 files changed, 39 insertions(+), 19 deletions(-)

da95c79803d90ce8eaf82b0c04dbde11c096ad87
diff --git a/diff.c b/diff.c
index c815918..bfc864d 100644
--- a/diff.c
+++ b/diff.c
@@ -504,9 +504,9 @@ static void prepare_temp_file(const char
 		}
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
-			char *buf, buf_[1024];
-			buf = ((sizeof(buf_) < st.st_size) ?
-			       xmalloc(st.st_size) : buf_);
+			char buf[PATH_MAX + 1]; /* ought to be SYMLINK_MAX */
+			if (sizeof(buf) <= st.st_size)
+				die("symlink too long: %s", name);
 			ret = readlink(name, buf, st.st_size);
 			if (ret < 0)
 				die("readlink(%s)", name);
diff --git a/diffcore-order.c b/diffcore-order.c
index b381223..0bc2b22 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -105,9 +105,13 @@ static int compare_pair_order(const void
 void diffcore_order(const char *orderfile)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	struct pair_order *o;
 	int i;
 
+	if (!q->nr)
+		return;
+
+	o = xmalloc(sizeof(*o) * q->nr);
 	prepare_order(orderfile);
 	for (i = 0; i < q->nr; i++) {
 		o[i].pair = q->queue[i];
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
index 68fe009..139fe88 100644
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -48,6 +48,9 @@ void diffcore_pathspec(const char **path
 	for (i = 0; pathspec[i]; i++)
 		;
 	speccnt = i;
+	if (!speccnt)
+		return;
+
 	spec = xmalloc(sizeof(*spec) * speccnt);
 	for (i = 0; pathspec[i]; i++) {
 		spec[i].spec = pathspec[i];
diff --git a/index-pack.c b/index-pack.c
index d4ce3af..541d7bc 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -352,18 +352,24 @@ static int sha1_compare(const void *_a, 
 static void write_index_file(const char *index_name, unsigned char *sha1)
 {
 	struct sha1file *f;
-	struct object_entry **sorted_by_sha =
-		xcalloc(nr_objects, sizeof(struct object_entry *));
-	struct object_entry **list = sorted_by_sha;
-	struct object_entry **last = sorted_by_sha + nr_objects;
+	struct object_entry **sorted_by_sha, **list, **last;
 	unsigned int array[256];
 	int i;
 	SHA_CTX ctx;
 
-	for (i = 0; i < nr_objects; ++i)
-		sorted_by_sha[i] = &objects[i];
-	qsort(sorted_by_sha, nr_objects, sizeof(sorted_by_sha[0]),
-	      sha1_compare);
+	if (nr_objects) {
+		sorted_by_sha =
+			xcalloc(nr_objects, sizeof(struct object_entry *));
+		list = sorted_by_sha;
+		last = sorted_by_sha + nr_objects;
+		for (i = 0; i < nr_objects; ++i)
+			sorted_by_sha[i] = &objects[i];
+		qsort(sorted_by_sha, nr_objects, sizeof(sorted_by_sha[0]),
+		      sha1_compare);
+
+	}
+	else
+		sorted_by_sha = list = last = NULL;
 
 	unlink(index_name);
 	f = sha1create("%s", index_name);
diff --git a/read-tree.c b/read-tree.c
index e3b9c0d..a46c6fe 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -294,17 +294,20 @@ static int unpack_trees(merge_fn_t fn)
 {
 	int indpos = 0;
 	unsigned len = object_list_length(trees);
-	struct tree_entry_list **posns = 
-		xmalloc(len * sizeof(struct tree_entry_list *));
+	struct tree_entry_list **posns;
 	int i;
 	struct object_list *posn = trees;
 	merge_size = len;
-	for (i = 0; i < len; i++) {
-		posns[i] = ((struct tree *) posn->item)->entries;
-		posn = posn->next;
+
+	if (len) {
+		posns = xmalloc(len * sizeof(struct tree_entry_list *));
+		for (i = 0; i < len; i++) {
+			posns[i] = ((struct tree *) posn->item)->entries;
+			posn = posn->next;
+		}
+		if (unpack_trees_rec(posns, len, "", fn, &indpos))
+			return -1;
 	}
-	if (unpack_trees_rec(posns, len, "", fn, &indpos))
-		return -1;
 
 	if (trivial_merges_only && nontrivial_merge)
 		die("Merge requires file-level merging");
diff --git a/tree-diff.c b/tree-diff.c
index 0ef06a9..382092b 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -263,6 +263,10 @@ void diff_tree_setup_paths(const char **
 
 		paths = p;
 		nr_paths = count_paths(paths);
+		if (nr_paths == 0) {
+			pathlens = NULL;
+			return;
+		}
 		pathlens = xmalloc(nr_paths * sizeof(int));
 		for (i=0; i<nr_paths; i++)
 			pathlens[i] = strlen(paths[i]);
-- 
1.0.GIT

Re: [PATCH] avoid asking ?alloc() for zero bytes.

[Johannes Schindelin]

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Avoid asking for zero bytes when that change simplifies overall
> logic.  Later we would change the wrapper to ask for 1 byte on
> platforms that return NULL for zero byte request.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> 
> ---
> 
>  * Here is what I have now that roughly corresponds to your
>    patch, which has been somewhat tested.

Thanks.

>    The final phase of making sure we return something from
>    x*alloc() is not done yet.

Tomorrow I'll compare the two patches and find out what, if anything, is 
needed.

Ciao,
Dscho

Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Johannes Schindelin]

Hi,

On Mon, 26 Dec 2005, Johannes Schindelin wrote:

> On Mon, 26 Dec 2005, Junio C Hamano wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > diff --git a/csum-file.c b/csum-file.c
> > > index 5f9249a..2c0f097 100644
> > > --- a/csum-file.c
> > > +++ b/csum-file.c
> > > @@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
> > >  	unsigned long maxsize;
> > >  	void *out;
> > >  
> > > +	if (size == 0)
> > > +		return 0;
> > > +
> > >  	memset(&stream, 0, sizeof(stream));
> > >  	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
> > >  	maxsize = deflateBound(&stream, size);
> > 
> > I think this and the one in sha1_file.c::write_sha1_file() are
> > wrong; 0-size input would not result in 0-size output.  Have you
> > tested them by actually exercising the codepaths you touched?
> 
> No, I did not test them.

Now I did. To make a long story short: this patch is wrong, wrong, wrong. 
Further, I think it is safe to assume that deflateBound() returns > 0.

Ciao,
Dscho

Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}

[Eric Wong]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> diff --git a/index-pack.c b/index-pack.c
> index d4ce3af..2927632 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -103,7 +103,7 @@ static void *unpack_entry_data(unsigned 
>  	unsigned long pack_limit = pack_size - 20;
>  	unsigned long pos = *current_pos;
>  	z_stream stream;
> -	void *buf = xmalloc(size);
> +	void *buf = xmalloc(size ? size : 1);
>  
>  	memset(&stream, 0, sizeof(stream));
>  	stream.next_out = buf;

This is the only one I've managed to hit upon further use the past week
of usage.  No way to break out of the function, either, because
current_pos needs to be updated (and it is changed despite a zero-byte
blob object being in my repository).

-- 
Eric Wong