# [PATCH 0/4] dietlibc compatibility 24 messages from 2005-12-24 to 2005-12-30. Participants: Eric Wong, Junio C Hamano, Johannes Schindelin, H. Peter Anvin, Linus Torvalds. Thread: https://gitlist.dev/t/2924 ## Eric Wong, 2005-12-24 12:10 Subject: [PATCH 0/4] dietlibc compatibility Message-ID: <20051224121007.GA19136@mail.yhbt.net> URL: https://gitlist.dev/e/20051224121007.GA19136%40mail.yhbt.net ``` 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, 2005-12-24 12:11 Subject: [PATCH 1/4] git.c: extra #include for dietlibc (and possibly other C libraries) Message-ID: <20051224121117.GB19136@mail.yhbt.net> URL: https://gitlist.dev/e/20051224121117.GB19136%40mail.yhbt.net In-Reply-To: <20051224121007.GA19136@mail.yhbt.net> ``` struct winsize is defined in , and that's not pulled in by other #includes in that file Signed-off-by: Eric Wong --- 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 #include #include +#include #include "git-compat-util.h" #ifndef PATH_MAX -- 1.0.GIT ``` ## Eric Wong, 2005-12-24 12:12 Subject: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: <20051224121243.GA3963@mail.yhbt.net> URL: https://gitlist.dev/e/20051224121243.GA3963%40mail.yhbt.net In-Reply-To: <20051224121007.GA19136@mail.yhbt.net> ``` 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 --- 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 ``` ## Eric Wong, 2005-12-24 12:13 Subject: [PATCH 3/4] add xmktime() function that always accounts for the TZ env Message-ID: <20051224121339.GB3963@mail.yhbt.net> URL: https://gitlist.dev/e/20051224121339.GB3963%40mail.yhbt.net In-Reply-To: <20051224121007.GA19136@mail.yhbt.net> ``` 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 --- 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 +#include #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 ``` ## Eric Wong, 2005-12-24 12:14 Subject: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: <20051224121454.GC3963@mail.yhbt.net> URL: https://gitlist.dev/e/20051224121454.GC3963%40mail.yhbt.net In-Reply-To: <20051224121007.GA19136@mail.yhbt.net> ``` 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 --- 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 ``` ## Johannes Schindelin, 2005-12-24 12:44 Subject: Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512241342100.5891%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051224121243.GA3963@mail.yhbt.net> ``` 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 ``` ## Johannes Schindelin, 2005-12-24 12:45 Subject: Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512241344200.5891%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051224121339.GB3963@mail.yhbt.net> ``` 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, 2005-12-24 18:28 Subject: Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: <7v3bkis631.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v3bkis631.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051224121454.GC3963@mail.yhbt.net> ``` Eric Wong 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; ``` ## Junio C Hamano, 2005-12-24 19:18 Subject: Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env Message-ID: <7vy82aqp5r.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vy82aqp5r.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051224121339.GB3963@mail.yhbt.net> ``` Eric Wong 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, 2005-12-24 19:52 Subject: Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env Message-ID: <20051224195247.GF3963@mail.yhbt.net> URL: https://gitlist.dev/e/20051224195247.GF3963%40mail.yhbt.net In-Reply-To: <7vy82aqp5r.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Eric Wong 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, 2005-12-24 21:10 Subject: Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env Message-ID: <7vpsnmqk0n.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vpsnmqk0n.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051224195247.GF3963@mail.yhbt.net> ``` Eric Wong writes: > Junio C Hamano wrote: >> Eric Wong 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, 2005-12-24 21:15 Subject: Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: <20051224211546.GG3963@mail.yhbt.net> URL: https://gitlist.dev/e/20051224211546.GG3963%40mail.yhbt.net In-Reply-To: <7v3bkis631.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Eric Wong 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 ``` ## Junio C Hamano, 2005-12-26 17:01 Subject: Re: [PATCH 1/4] git.c: extra #include for dietlibc (and possibly other C libraries) Message-ID: <7v3bkflrmf.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v3bkflrmf.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051224121117.GB19136@mail.yhbt.net> ``` Eric Wong writes: > struct winsize is defined in , 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. ``` ## Johannes Schindelin, 2005-12-26 18:16 Subject: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512261916440.8435%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051224211546.GG3963@mail.yhbt.net> ``` 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 --- 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 URL: https://gitlist.dev/e/7vzmmnisix.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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; } ``` ## Johannes Schindelin, 2005-12-26 20:34 Subject: Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512262134290.19331%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vzmmnisix.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Mon, 26 Dec 2005, Junio C Hamano wrote: > Johannes Schindelin 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 ``` ## Junio C Hamano, 2005-12-26 22:03 Subject: [PATCH] avoid asking ?alloc() for zero bytes. Message-ID: <7vslsfikii.fsf_-_@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vslsfikii.fsf_-_%40assigned-by-dhcp.cox.net In-Reply-To: ``` 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 --- * 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 URL: https://gitlist.dev/e/Pine.LNX.4.63.0512262316150.32161%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vslsfikii.fsf_-_@assigned-by-dhcp.cox.net> ``` 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 > > --- > > * 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 ``` ## H. Peter Anvin, 2005-12-28 04:22 Subject: Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: <43B21312.50603@zytor.com> URL: https://gitlist.dev/e/43B21312.50603%40zytor.com In-Reply-To: <20051224121243.GA3963@mail.yhbt.net> ``` 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 ``` ## Linus Torvalds, 2005-12-28 04:38 Subject: Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0512272036380.14098%40g5.osdl.org In-Reply-To: <43B21312.50603@zytor.com> ``` 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, 2005-12-28 05:07 Subject: Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: <7vpsnh24j3.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vpsnh24j3.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds 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; ``` ## H. Peter Anvin, 2005-12-28 16:58 Subject: Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes Message-ID: <43B2C44B.1070102@zytor.com> URL: https://gitlist.dev/e/43B2C44B.1070102%40zytor.com In-Reply-To: ``` 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 ``` ## Johannes Schindelin, 2005-12-28 20:38 Subject: Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512282134380.2236%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Hi, On Mon, 26 Dec 2005, Johannes Schindelin wrote: > On Mon, 26 Dec 2005, Junio C Hamano wrote: > > > Johannes Schindelin 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, 2005-12-30 23:00 Subject: Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc} Message-ID: <20051230230035.GA7165@mail.yhbt.net> URL: https://gitlist.dev/e/20051230230035.GA7165%40mail.yhbt.net In-Reply-To: ``` Johannes Schindelin 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 ```