Re: [PATCH] Use ALLOC_GROW() instead of inline code
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:05
"Dmitry S. Dolzhenko" [off-list ref] writes:
quoted hunk
diff --git a/dir.c b/dir.c index b35b633..72f6e2a 100644 --- a/dir.c +++ b/dir.c@@ -1329,13 +1329,10 @@ static struct path_simplify *create_simplify(const char **pathspec) for (nr = 0 ; ; nr++) { const char *match; - if (nr >= alloc) { - alloc = alloc_nr(alloc); - simplify = xrealloc(simplify, alloc * sizeof(*simplify)); - } match = *pathspec++; if (!match) break; + ALLOC_GROW(simplify, nr + 1, alloc); simplify[nr].path = match; simplify[nr].len = simple_length(match); }
What follows the post-context of this hunk is a NULL termination of
the array:
simplify[nr].path = NULL;
simplify[nr].len = 0;
If the first element in pathspec[] were NULL, we set nr to 0, break
the loop without calling ALLOC_GROW() even once, and try to NULL
terminate simplify[] array after the loop.
Don't we try to store to an unallocated piece of memory with this
change?
quoted hunk
diff --git a/read-cache.c b/read-cache.c index 33dd676..e585541 100644 --- a/read-cache.c +++ b/read-cache.c@@ -1466,8 +1462,7 @@ int read_index_from(struct index_state *istate, const char *path) istate->version = ntohl(hdr->hdr_version); istate->cache_nr = ntohl(hdr->hdr_entries); - istate->cache_alloc = alloc_nr(istate->cache_nr); - istate->cache = xcalloc(istate->cache_alloc, sizeof(*istate->cache)); + ALLOC_GROW(istate->cache, istate->cache_nr, istate->cache_alloc);
This being the initial allocation, not growing reallocation, use of ALLOC_GROW() looks somewhat strange. I know that an realloc from NULL ends up being the same as calloc(), but still.