git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Use ALLOC_GROW() instead of inline code

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 27, 2014, 22:11 UTC
Message-ID
<530FB82C.70005@alum.mit.edu>
In-Reply-To
<530FA3E7.8020200@yandex.ru>
Dmitry,

That's cool; I never imagined there would be so many sites that could be cleaned up in this way.

In my opinion, it would be preferable for this patch to be broken into multiple commits, one for each site (or each file, if a file has multiple sites that are logically related). That would make it easier to review the patches and easier to bisect if we later find a problem. Please make sure that you note if there are any sites where the rewritten code doesn't have exactly the same semantics as the original (I don't know if there are sites like this, but if there are...). That helps reviewers focus on the changes that might be "controversial".

[Please leave the other microprojects for other students (I just wrote an email on this topic).]

Thanks, Michael

On 02/27/2014 09:45 PM, Dmitry S. Dolzhenko wrote:
Show 278 quoted lines
> Signed-off-by: Dmitry S. Dolzhenko <dmitrys.dolzhenko@gmail.com>
> ---
>  attr.c                 |  7 +------
>  builtin/pack-objects.c |  7 +------
>  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           |  9 ++-------
>  reflog-walk.c          | 13 +++----------
>  replace_object.c       |  8 ++------
>  12 files changed, 19 insertions(+), 79 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;
>  }
>  
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 541667f..92cbce8 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1156,12 +1156,7 @@ 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,
> 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++;
> 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);
> 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,
> diff --git a/diff.c b/diff.c
> index 8e4a6a9..f5f0fd1 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;
>  }
>  
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 6c7a72f..f54d5bf 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,
> 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);
>  	}
> 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));
> diff --git a/read-cache.c b/read-cache.c
> index 33dd676..e585541 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -993,11 +993,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++;
> @@ -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);
>  	istate->initialized = 1;
>  
>  	if (istate->version == 4)
> diff --git a/reflog-walk.c b/reflog-walk.c
> index b2fbdb2..879d2ed 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,8 @@ 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;
> 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,
> 
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Previous: Dmitry S. DolzhenkoNext: Junio C Hamano
Message 2 of 42 in “Use ALLOC_GROW() instead of inline code”
  1. Use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Feb 27, 2014
  2. Michael HaggertyFeb 27, 2014
  3. Junio C HamanoFeb 27, 2014
  4. 00/11 Use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Feb 28, 2014
  5. 01/11 builtin/pack-objects.c: change check_pbase_path() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  6. Duy NguyenFeb 28, 2014
  7. Duy NguyenFeb 28, 2014
  8. Michael HaggertyFeb 28, 2014
  9. Duy NguyenFeb 28, 2014
  10. Junio C HamanoFeb 28, 2014
  11. Jeff KingMar 1, 2014
  12. Junio C HamanoMar 3, 2014
  13. 02/11 bundle.c: change add_to_ref_list() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  14. 03/11 cache-tree.c: change find_subtree() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  15. 04/11 commit.c: change register_commit_graft() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  16. 05/11 diff.c: use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Feb 28, 2014
  17. 06/11 diffcore-rename.c: use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Feb 28, 2014
  18. 07/11 patch-ids.c: change add_commit() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  19. 08/11 replace_object.c: change register_replace_object() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  20. 09/11 reflog-walk.c: use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Feb 28, 2014
  21. Duy NguyenFeb 28, 2014
  22. Junio C HamanoFeb 28, 2014
  23. 10/11 dir.c: change create_simplify() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  24. 11/11 attr.c: change handle_attr_line() to use ALLOC_GROW()Dmitry S. Dolzhenko, Feb 28, 2014
  25. Michael HaggertyFeb 28, 2014
  26. Dmitry S. DolzhenkoMar 1, 2014
  27. Junio C HamanoMar 3, 2014
  28. 00/11 Use ALLOC_GROW() instead of inline codeDmitry S. Dolzhenko, Mar 3, 2014
  29. 01/11 builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()Dmitry S. Dolzhenko, Mar 3, 2014
  30. 02/11 bundle.c: use ALLOC_GROW() in add_to_ref_list()Dmitry S. Dolzhenko, Mar 3, 2014
  31. 03/11 cache-tree.c: use ALLOC_GROW() in find_subtree()Dmitry S. Dolzhenko, Mar 3, 2014
  32. 04/11 commit.c: use ALLOC_GROW() in register_commit_graft()Dmitry S. Dolzhenko, Mar 3, 2014
  33. 05/11 diff.c: use ALLOC_GROW()Dmitry S. Dolzhenko, Mar 3, 2014
  34. 06/11 diffcore-rename.c: use ALLOC_GROW()Dmitry S. Dolzhenko, Mar 3, 2014
  35. 07/11 patch-ids.c: use ALLOC_GROW() in add_commit()Dmitry S. Dolzhenko, Mar 3, 2014
  36. 08/11 replace_object.c: use ALLOC_GROW() in register_replace_object()Dmitry S. Dolzhenko, Mar 3, 2014
  37. 09/11 reflog-walk.c: use ALLOC_GROW()Dmitry S. Dolzhenko, Mar 3, 2014
  38. 10/11 dir.c: use ALLOC_GROW() in create_simplify()Dmitry S. Dolzhenko, Mar 3, 2014
  39. 11/11 attr.c: use ALLOC_GROW() in handle_attr_line()Dmitry S. Dolzhenko, Mar 3, 2014
  40. Eric SunshineMar 3, 2014
  41. Junio C HamanoMar 3, 2014
  42. He SunMar 3, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.