threads / patch / 2924

patch, 4 partsdietlibc compatibility

Subject: [PATCH 0/4] dietlibc compatibility

## tl;dr

24 messages between Dec 24, 2005 and Dec 30, 2005. Diffs are folded; open one to read it.

replies: 23people: 5as markdown or json

Eric Wong· Dec 24, 2005, 12:10 UTC · lore

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
Eric Wong· Dec 24, 2005, 12:11 UTC · re: Eric Wong · lore

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

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
Show changes to git.c +1 −0
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
Junio C Hamano· Dec 26, 2005, 17:01 UTC · re: Eric Wong · lore

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

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.

Eric Wong· Dec 24, 2005, 12:12 UTC · re: Eric Wong · lore

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

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
Show changes to 2 files +4 −1

commit.c, diffcore-rename.c

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
Johannes Schindelin· Dec 24, 2005, 12:44 UTC · re: Eric Wong · lore

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

Hi,
On Sat, 24 Dec 2005, Eric Wong wrote:
Show 8 quoted lines
> 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

H. Peter Anvin· Dec 28, 2005, 04:22 UTC · re: Eric Wong · lore

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

Eric Wong wrote:
Show 9 quoted lines
> 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
Linus Torvalds· Dec 28, 2005, 04:38 UTC · re: H. Peter Anvin · lore

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

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
Junio C Hamano· Dec 28, 2005, 05:07 UTC · re: Linus Torvalds · lore

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

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.

---
Show changes to git-compat-util.h +6 −0
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;
H. Peter Anvin· Dec 28, 2005, 16:58 UTC · re: Linus Torvalds · lore

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

Linus Torvalds wrote:
Show 14 quoted lines
> 
> 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
Eric Wong· Dec 24, 2005, 12:13 UTC · re: Eric Wong · lore

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

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
Show changes to 3 files +30 −6

cache.h, convert-objects.c, date.c

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
Johannes Schindelin· Dec 24, 2005, 12:45 UTC · re: Eric Wong · lore

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

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

Junio C Hamano· Dec 24, 2005, 19:18 UTC · re: Eric Wong · lore

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

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?
Eric Wong· Dec 24, 2005, 19:52 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <junkio@cox.net> wrote:
Show 7 quoted lines
> 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
Junio C Hamano· Dec 24, 2005, 21:10 UTC · re: Eric Wong · lore

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

Eric Wong <normalperson@yhbt.net> writes:
Show 10 quoted lines
> 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.

Eric Wong· Dec 24, 2005, 12:14 UTC · re: Eric Wong · lore

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

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
Show changes to git-compat-util.h +20 −3
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
Junio C Hamano· Dec 24, 2005, 18:28 UTC · re: Eric Wong · lore

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

Eric Wong <normalperson@yhbt.net> writes:
Show 7 quoted lines
> 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 --
Show changes to git-compat-util.h +10 −0
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;
Eric Wong· Dec 24, 2005, 21:15 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <junkio@cox.net> wrote:
Show 21 quoted lines
> 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.

Show changes to git-compat-util.h +50 −3
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
Johannes Schindelin· Dec 26, 2005, 18:16 UTC · re: Eric Wong · lore

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

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
Show changes to 10 files +76 −49

csum-file.c, diff.c, diffcore-order.c, diffcore-pathspec.c, index-pack.c, pack-objects.c, read-tree.c, sha1_file.c, tree-diff.c, unpack-objects.c

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
Junio C Hamano· Dec 26, 2005, 19:10 UTC · re: Johannes Schindelin · lore

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

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.
Show 14 quoted lines
> 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?

Show 5 quoted lines
> 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.

Show 10 quoted lines
> @@ -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.

Show 13 quoted lines
> @@ -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;
        }
Johannes Schindelin· Dec 26, 2005, 20:34 UTC · re: Junio C Hamano · lore

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

Hi,
On Mon, 26 Dec 2005, Junio C Hamano wrote:
Show 20 quoted lines
> 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.

Show 8 quoted lines
> > 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.

Show 14 quoted lines
> > @@ -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.

Show 16 quoted lines
> > @@ -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.
Show 6 quoted lines
> 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.

Show 13 quoted lines
>  - 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

Junio C Hamano· Dec 26, 2005, 22:03 UTC · re: Johannes Schindelin · lore

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

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
Show changes to 6 files +39 −19

diff.c, diffcore-order.c, diffcore-pathspec.c, index-pack.c, read-tree.c, tree-diff.c

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
Johannes Schindelin· Dec 26, 2005, 22:18 UTC · re: Junio C Hamano · lore

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

Hi,
On Mon, 26 Dec 2005, Junio C Hamano wrote:
Show 10 quoted lines
> 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

Johannes Schindelin· Dec 28, 2005, 20:38 UTC · re: Johannes Schindelin · lore

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

Hi,
On Mon, 26 Dec 2005, Johannes Schindelin wrote:
Show 24 quoted lines
> 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

Eric Wong· Dec 30, 2005, 23:00 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 13 quoted lines
> 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

← back to recent threads