[PATCH v4 00/14] Use ALLOC_GROW() instead of inline code

[PATCH v4 00/14] Use ALLOC_GROW() instead of inline code

[Dmitry S. Dolzhenko]

This version differs from previous [1] the following changes:
  - added three new commits with similar changes in "builtin/mktree.c",
    "cache-tree.c" and "sha1_file.c".
  - updated commit messages: "use ALLOC_GROW() in function_name()" instead of
    "change function_name() to use ALLOC_GROW()"
  - updated [PATCH v2 01/11] [2] to keep code lines within 80 columns in 
    "builtin/pack-objects.c"

Duy Nguyen, Michael Haggerty, Junio C Hamano, Eric Sunshine, and He Sun, 
thanks you very much for your remarks and advices

[1] http://thread.gmane.org/gmane.comp.version-control.git/242919
[2] http://thread.gmane.org/gmane.comp.version-control.git/242920

Dmitry S. Dolzhenko (14):
  builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
  bundle.c: use ALLOC_GROW() in add_to_ref_list()
  cache-tree.c: use ALLOC_GROW() in find_subtree()
  commit.c: use ALLOC_GROW() in register_commit_graft()
  diff.c: use ALLOC_GROW()
  diffcore-rename.c: use ALLOC_GROW()
  patch-ids.c: use ALLOC_GROW() in add_commit()
  replace_object.c: use ALLOC_GROW() in register_replace_object()
  reflog-walk.c: use ALLOC_GROW()
  dir.c: use ALLOC_GROW() in create_simplify()
  attr.c: use ALLOC_GROW() in handle_attr_line()
  builtin/mktree.c: use ALLOC_GROW() in append_to_tree()
  read-cache.c: use ALLOC_GROW() in add_index_entry()
  sha1_file.c: use ALLOC_GROW() in pretend_sha1_file()

 attr.c                 |  7 +------
 builtin/mktree.c       |  5 +----
 builtin/pack-objects.c |  9 +++------
 bundle.c               |  6 +-----
 cache-tree.c           |  6 +-----
 commit.c               |  8 ++------
 diff.c                 | 12 ++----------
 diffcore-rename.c      | 12 ++----------
 dir.c                  |  5 +----
 patch-ids.c            |  5 +----
 read-cache.c           |  6 +-----
 reflog-walk.c          | 12 ++----------
 replace_object.c       |  8 ++------
 sha1_file.c            |  7 +------
 14 files changed, 21 insertions(+), 87 deletions(-)

-- 
1.8.3.2

[PATCH v4 01/14] builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 builtin/pack-objects.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..0ffad6f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1213,12 +1213,9 @@ static int check_pbase_path(unsigned hash)
 	if (0 <= pos)
 		return 1;
 	pos = -pos - 1;
-	if (done_pbase_paths_alloc <= done_pbase_paths_num) {
-		done_pbase_paths_alloc = alloc_nr(done_pbase_paths_alloc);
-		done_pbase_paths = xrealloc(done_pbase_paths,
-					    done_pbase_paths_alloc *
-					    sizeof(unsigned));
-	}
+	ALLOC_GROW(done_pbase_paths,
+		   done_pbase_paths_num + 1,
+		   done_pbase_paths_alloc);
 	done_pbase_paths_num++;
 	if (pos < done_pbase_paths_num)
 		memmove(done_pbase_paths + pos + 1,
-- 
1.8.3.2

[PATCH v4 02/14] bundle.c: use ALLOC_GROW() in add_to_ref_list()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 bundle.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/bundle.c b/bundle.c
index e99065c..1388a3e 100644
--- a/bundle.c
+++ b/bundle.c
@@ -14,11 +14,7 @@ static const char bundle_signature[] = "# v2 git bundle\n";
 static void add_to_ref_list(const unsigned char *sha1, const char *name,
 		struct ref_list *list)
 {
-	if (list->nr + 1 >= list->alloc) {
-		list->alloc = alloc_nr(list->nr + 1);
-		list->list = xrealloc(list->list,
-				list->alloc * sizeof(list->list[0]));
-	}
+	ALLOC_GROW(list->list, list->nr + 1, list->alloc);
 	memcpy(list->list[list->nr].sha1, sha1, 20);
 	list->list[list->nr].name = xstrdup(name);
 	list->nr++;
-- 
1.8.3.2

[PATCH v4 03/14] cache-tree.c: use ALLOC_GROW() in find_subtree()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 cache-tree.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..30149d1 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -75,11 +75,7 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it,
 		return NULL;
 
 	pos = -pos-1;
-	if (it->subtree_alloc <= it->subtree_nr) {
-		it->subtree_alloc = alloc_nr(it->subtree_alloc);
-		it->down = xrealloc(it->down, it->subtree_alloc *
-				    sizeof(*it->down));
-	}
+	ALLOC_GROW(it->down, it->subtree_nr + 1, it->subtree_alloc);
 	it->subtree_nr++;
 
 	down = xmalloc(sizeof(*down) + pathlen + 1);
-- 
1.8.3.2

[PATCH v4 04/14] commit.c: use ALLOC_GROW() in register_commit_graft()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 commit.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..e004314 100644
--- a/commit.c
+++ b/commit.c
@@ -147,12 +147,8 @@ int register_commit_graft(struct commit_graft *graft, int ignore_dups)
 		return 1;
 	}
 	pos = -pos - 1;
-	if (commit_graft_alloc <= ++commit_graft_nr) {
-		commit_graft_alloc = alloc_nr(commit_graft_alloc);
-		commit_graft = xrealloc(commit_graft,
-					sizeof(*commit_graft) *
-					commit_graft_alloc);
-	}
+	ALLOC_GROW(commit_graft, commit_graft_nr + 1, commit_graft_alloc);
+	commit_graft_nr++;
 	if (pos < commit_graft_nr)
 		memmove(commit_graft + pos + 1,
 			commit_graft + pos,
-- 
1.8.3.2

[PATCH v4 05/14] diff.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
diffstat_add() and diff_q()

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 diff.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index e800666..aebdfda 100644
--- a/diff.c
+++ b/diff.c
@@ -1361,11 +1361,7 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
 {
 	struct diffstat_file *x;
 	x = xcalloc(sizeof (*x), 1);
-	if (diffstat->nr == diffstat->alloc) {
-		diffstat->alloc = alloc_nr(diffstat->alloc);
-		diffstat->files = xrealloc(diffstat->files,
-				diffstat->alloc * sizeof(x));
-	}
+	ALLOC_GROW(diffstat->files, diffstat->nr + 1, diffstat->alloc);
 	diffstat->files[diffstat->nr++] = x;
 	if (name_b) {
 		x->from_name = xstrdup(name_a);
@@ -3965,11 +3961,7 @@ struct diff_queue_struct diff_queued_diff;
 
 void diff_q(struct diff_queue_struct *queue, struct diff_filepair *dp)
 {
-	if (queue->alloc <= queue->nr) {
-		queue->alloc = alloc_nr(queue->alloc);
-		queue->queue = xrealloc(queue->queue,
-					sizeof(dp) * queue->alloc);
-	}
+	ALLOC_GROW(queue->queue, queue->nr + 1, queue->alloc);
 	queue->queue[queue->nr++] = dp;
 }
 
-- 
1.8.3.2

Re: [PATCH v4 05/14] diff.c: use ALLOC_GROW()

[Junio C Hamano]

"Dmitry S. Dolzhenko" <dmitrys.dolzhenko@yandex.ru> writes:

> Use ALLOC_GROW() instead inline code in
> diffstat_add() and diff_q()

"...instead of open coding it in..." may read better.

>
> Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
> ---
>  diff.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index e800666..aebdfda 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1361,11 +1361,7 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
>  {
>  	struct diffstat_file *x;
>  	x = xcalloc(sizeof (*x), 1);
> -	if (diffstat->nr == diffstat->alloc) {
> -		diffstat->alloc = alloc_nr(diffstat->alloc);
> -		diffstat->files = xrealloc(diffstat->files,
> -				diffstat->alloc * sizeof(x));
> -	}
> +	ALLOC_GROW(diffstat->files, diffstat->nr + 1, diffstat->alloc);
>  	diffstat->files[diffstat->nr++] = x;
>  	if (name_b) {
>  		x->from_name = xstrdup(name_a);
> @@ -3965,11 +3961,7 @@ struct diff_queue_struct diff_queued_diff;
>  
>  void diff_q(struct diff_queue_struct *queue, struct diff_filepair *dp)
>  {
> -	if (queue->alloc <= queue->nr) {
> -		queue->alloc = alloc_nr(queue->alloc);
> -		queue->queue = xrealloc(queue->queue,
> -					sizeof(dp) * queue->alloc);
> -	}
> +	ALLOC_GROW(queue->queue, queue->nr + 1, queue->alloc);
>  	queue->queue[queue->nr++] = dp;
>  }

[PATCH v4 06/14] diffcore-rename.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
locate_rename_dst() and register_rename_src()

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 diffcore-rename.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 9b4f068..fbf3272 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -38,11 +38,7 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
 	if (!insert_ok)
 		return NULL;
 	/* insert to make it at "first" */
-	if (rename_dst_alloc <= rename_dst_nr) {
-		rename_dst_alloc = alloc_nr(rename_dst_alloc);
-		rename_dst = xrealloc(rename_dst,
-				      rename_dst_alloc * sizeof(*rename_dst));
-	}
+	ALLOC_GROW(rename_dst, rename_dst_nr + 1, rename_dst_alloc);
 	rename_dst_nr++;
 	if (first < rename_dst_nr)
 		memmove(rename_dst + first + 1, rename_dst + first,
@@ -82,11 +78,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filepair *p)
 	}
 
 	/* insert to make it at "first" */
-	if (rename_src_alloc <= rename_src_nr) {
-		rename_src_alloc = alloc_nr(rename_src_alloc);
-		rename_src = xrealloc(rename_src,
-				      rename_src_alloc * sizeof(*rename_src));
-	}
+	ALLOC_GROW(rename_src, rename_src_nr + 1, rename_src_alloc);
 	rename_src_nr++;
 	if (first < rename_src_nr)
 		memmove(rename_src + first + 1, rename_src + first,
-- 
1.8.3.2

[PATCH v4 07/14] patch-ids.c: use ALLOC_GROW() in add_commit()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 patch-ids.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/patch-ids.c b/patch-ids.c
index bc8a28f..bf81b92 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -83,10 +83,7 @@ static struct patch_id *add_commit(struct commit *commit,
 	ent = &bucket->bucket[bucket->nr++];
 	hashcpy(ent->patch_id, sha1);
 
-	if (ids->alloc <= ids->nr) {
-		ids->alloc = alloc_nr(ids->nr);
-		ids->table = xrealloc(ids->table, sizeof(ent) * ids->alloc);
-	}
+	ALLOC_GROW(ids->table, ids->nr + 1, ids->alloc);
 	if (pos < ids->nr)
 		memmove(ids->table + pos + 1, ids->table + pos,
 			sizeof(ent) * (ids->nr - pos));
-- 
1.8.3.2

[PATCH v4 08/14] replace_object.c: use ALLOC_GROW() in register_replace_object()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 replace_object.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index cdcaf8c..843deef 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -36,12 +36,8 @@ static int register_replace_object(struct replace_object *replace,
 		return 1;
 	}
 	pos = -pos - 1;
-	if (replace_object_alloc <= ++replace_object_nr) {
-		replace_object_alloc = alloc_nr(replace_object_alloc);
-		replace_object = xrealloc(replace_object,
-					  sizeof(*replace_object) *
-					  replace_object_alloc);
-	}
+	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
+	replace_object_nr++;
 	if (pos < replace_object_nr)
 		memmove(replace_object + pos + 1,
 			replace_object + pos,
-- 
1.8.3.2

[PATCH v4 09/14] reflog-walk.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
add_commit_info() and read_one_reflog()

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 reflog-walk.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index b2fbdb2..2899729 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -26,11 +26,7 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 	struct complete_reflogs *array = cb_data;
 	struct reflog_info *item;
 
-	if (array->nr >= array->alloc) {
-		array->alloc = alloc_nr(array->nr + 1);
-		array->items = xrealloc(array->items, array->alloc *
-			sizeof(struct reflog_info));
-	}
+	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
 	memcpy(item->osha1, osha1, 20);
 	memcpy(item->nsha1, nsha1, 20);
@@ -114,11 +110,7 @@ static void add_commit_info(struct commit *commit, void *util,
 		struct commit_info_lifo *lifo)
 {
 	struct commit_info *info;
-	if (lifo->nr >= lifo->alloc) {
-		lifo->alloc = alloc_nr(lifo->nr + 1);
-		lifo->items = xrealloc(lifo->items,
-			lifo->alloc * sizeof(struct commit_info));
-	}
+	ALLOC_GROW(lifo->items, lifo->nr + 1, lifo->alloc);
 	info = lifo->items + lifo->nr;
 	info->commit = commit;
 	info->util = util;
-- 
1.8.3.2

[PATCH v4 10/14] dir.c: use ALLOC_GROW() in create_simplify()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 dir.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index 98bb50f..4ae38e4 100644
--- a/dir.c
+++ b/dir.c
@@ -1341,10 +1341,7 @@ 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));
-		}
+		ALLOC_GROW(simplify, nr + 1, alloc);
 		match = *pathspec++;
 		if (!match)
 			break;
-- 
1.8.3.2

[PATCH v4 11/14] attr.c: use ALLOC_GROW() in handle_attr_line()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 attr.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/attr.c b/attr.c
index 8d13d70..734222d 100644
--- a/attr.c
+++ b/attr.c
@@ -338,12 +338,7 @@ static void handle_attr_line(struct attr_stack *res,
 	a = parse_attr_line(line, src, lineno, macro_ok);
 	if (!a)
 		return;
-	if (res->alloc <= res->num_matches) {
-		res->alloc = alloc_nr(res->num_matches);
-		res->attrs = xrealloc(res->attrs,
-				      sizeof(struct match_attr *) *
-				      res->alloc);
-	}
+	ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
 	res->attrs[res->num_matches++] = a;
 }
 
-- 
1.8.3.2

[PATCH v4 12/14] builtin/mktree.c: use ALLOC_GROW() in append_to_tree()

[Dmitry S. Dolzhenko]

Helped-by: He Sun <sunheehnus@gmail.com>
Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 builtin/mktree.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/builtin/mktree.c b/builtin/mktree.c
index f92ba40..a964d6b 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -23,10 +23,7 @@ static void append_to_tree(unsigned mode, unsigned char *sha1, char *path)
 	if (strchr(path, '/'))
 		die("path %s contains slash", path);
 
-	if (alloc <= used) {
-		alloc = alloc_nr(used);
-		entries = xrealloc(entries, sizeof(*entries) * alloc);
-	}
+	ALLOC_GROW(entries, used + 1, alloc);
 	ent = entries[used++] = xmalloc(sizeof(**entries) + len + 1);
 	ent->mode = mode;
 	ent->len = len;
-- 
1.8.3.2

[PATCH v4 13/14] read-cache.c: use ALLOC_GROW() in add_index_entry()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 read-cache.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index fb440b4..cbdf954 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -990,11 +990,7 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
 	}
 
 	/* Make sure the array is big enough .. */
-	if (istate->cache_nr == istate->cache_alloc) {
-		istate->cache_alloc = alloc_nr(istate->cache_alloc);
-		istate->cache = xrealloc(istate->cache,
-					istate->cache_alloc * sizeof(*istate->cache));
-	}
+	ALLOC_GROW(istate->cache, istate->cache_nr + 1, istate->cache_alloc);
 
 	/* Add it in.. */
 	istate->cache_nr++;
-- 
1.8.3.2

[PATCH v4 14/14] sha1_file.c: use ALLOC_GROW() in pretend_sha1_file()

[Dmitry S. Dolzhenko]

Helped-by: He Sun <sunheehnus@gmail.com>
Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@yandex.ru>
---
 sha1_file.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 019628a..3cb17b8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2624,12 +2624,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
 	hash_sha1_file(buf, len, typename(type), sha1);
 	if (has_sha1_file(sha1) || find_cached_object(sha1))
 		return 0;
-	if (cached_object_alloc <= cached_object_nr) {
-		cached_object_alloc = alloc_nr(cached_object_alloc);
-		cached_objects = xrealloc(cached_objects,
-					  sizeof(*cached_objects) *
-					  cached_object_alloc);
-	}
+	ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
 	co = &cached_objects[cached_object_nr++];
 	co->size = len;
 	co->type = type;
-- 
1.8.3.2

Re: [PATCH v4 00/14] Use ALLOC_GROW() instead of inline code

[Junio C Hamano]

> Dmitry S. Dolzhenko (14):
>   builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
>   bundle.c: use ALLOC_GROW() in add_to_ref_list()
>   cache-tree.c: use ALLOC_GROW() in find_subtree()
>   commit.c: use ALLOC_GROW() in register_commit_graft()
>   diff.c: use ALLOC_GROW()
>   diffcore-rename.c: use ALLOC_GROW()
>   patch-ids.c: use ALLOC_GROW() in add_commit()
>   replace_object.c: use ALLOC_GROW() in register_replace_object()
>   reflog-walk.c: use ALLOC_GROW()
>   dir.c: use ALLOC_GROW() in create_simplify()
>   attr.c: use ALLOC_GROW() in handle_attr_line()
>   builtin/mktree.c: use ALLOC_GROW() in append_to_tree()
>   read-cache.c: use ALLOC_GROW() in add_index_entry()
>   sha1_file.c: use ALLOC_GROW() in pretend_sha1_file()

All looked cleanly done.

The resulting code of 1, 3, 4, 6 and 8 share this pattern:

	ALLOC_GROW(table, number + 1, alloc);
        number++;

which may be easier to understand if done the other way around:

        number++;
	ALLOC_GROW(table, number, alloc);

That is, "we know we want one more, so make sure they fit in the
table".

But that is just a minor issue; I suspect many existing callsites to
ALLOC_GROW() already follow the former pattern, and if we decide to
to switch the former to the latter, we shouldn't be doing so within
this series (we should do that as a separate series on top of this).

Thanks; will queue.