Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:15
Johannes Schindelin [off-list ref] 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.
quoted hunk
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?
quoted hunk
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.
quoted hunk
@@ -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.
quoted hunk
@@ -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;
}