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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 27, 2014, 22:44 UTC
Message-ID
<xmqqr46ow4zs.fsf@gitster.dls.corp.google.com>
In-Reply-To
<530FA3E7.8020200@yandex.ru>
"Dmitry S. Dolzhenko" <dmitrys.dolzhenko@yandex.ru> writes:
Show 19 quoted lines
> 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?

Show 11 quoted lines
> 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.

Previous: Michael HaggertyNext: Dmitry S. Dolzhenko
Message 3 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.