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

Re: [PATCH 03/15] do not create "struct commit" with xcalloc

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 10, 2014, 21:35 UTC
Message-ID
<xmqqbnu08n2j.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140609181008.GC20315@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> In both blame and merge-recursive, we sometimes create a
> "fake" commit struct for convenience (e.g., to represent the
> HEAD state as if we would commit it). By allocating
> ourselves rather than using alloc_commit_node, we do not
> properly set the "index" field of the commit. This can
> produce subtle bugs if we then use commit-slab on the
> resulting commit, as we will share the "0" index with
> another commit.

The change I see here is all good, but I wonder if it is too late to start the real index from 1 and catch anything that has 0 index with a BUG("do not hand-allocate a commit").

Show 37 quoted lines
> We can fix this by using alloc_commit_node() to allocate.
> Note that we cannot free the result, as it is part of our
> commit allocator. However, both cases were already leaking
> the allocated commit anyway, so there's nothing to fix up.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/blame.c   | 2 +-
>  merge-recursive.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index a52a279..d6056a5 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
>  	struct strbuf msg = STRBUF_INIT;
>  
>  	time(&now);
> -	commit = xcalloc(1, sizeof(*commit));
> +	commit = alloc_commit_node();
>  	commit->object.parsed = 1;
>  	commit->date = now;
>  	commit->object.type = OBJ_COMMIT;
> diff --git a/merge-recursive.c b/merge-recursive.c
> index cab16fa..2b37d42 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -40,7 +40,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,
>  
>  static struct commit *make_virtual_commit(struct tree *tree, const char *comment)
>  {
> -	struct commit *commit = xcalloc(1, sizeof(struct commit));
> +	struct commit *commit = alloc_commit_node();
>  	struct merge_remote_desc *desc = xmalloc(sizeof(*desc));
>  
>  	desc->name = comment;
Previous: Jeff KingNext: Jeff King
Message 6 of 60 in “store length of commit->buffer”
  1. 0/15 store length of commit->bufferJeff King, Jun 9, 2014
  2. 01/15 alloc: include any-object allocations in alloc_reportJeff King, Jun 9, 2014
  3. 02/15 commit: push commit_index update into alloc_commit_nodeJeff King, Jun 9, 2014
  4. Junio C HamanoJun 10, 2014
  5. 03/15 do not create "struct commit" with xcallocJeff King, Jun 9, 2014
  6. Junio C HamanoJun 10, 2014
  7. Jeff KingJun 10, 2014
  8. 04/15 logmsg_reencode: return const bufferJeff King, Jun 9, 2014
  9. Eric SunshineJun 10, 2014
  10. 05/15 sequencer: use logmsg_reencode in get_messageJeff King, Jun 9, 2014
  11. Junio C HamanoJun 10, 2014
  12. Jeff KingJun 10, 2014
  13. 06/15 provide a helper to free commit bufferJeff King, Jun 9, 2014
  14. 07/15 provide a helper to set the commit bufferJeff King, Jun 9, 2014
  15. 08/15 provide helpers to access the commit bufferJeff King, Jun 9, 2014
  16. Eric SunshineJun 10, 2014
  17. 09/15 use get_cached_commit_buffer where appropriateJeff King, Jun 9, 2014
  18. 10/15 use get_commit_buffer to avoid duplicate codeJeff King, Jun 9, 2014
  19. Eric SunshineJun 10, 2014
  20. Jeff KingJun 10, 2014
  21. 11/15 convert logmsg_reencode to get_commit_bufferJeff King, Jun 9, 2014
  22. 12/15 use get_commit_buffer everywhereJeff King, Jun 9, 2014
  23. Junio C HamanoJun 9, 2014
  24. Jeff KingJun 10, 2014
  25. Jeff KingJun 10, 2014
  26. Junio C HamanoJun 10, 2014
  27. Jeff KingJun 10, 2014
  28. 13/15 commit-slab: provide a static initializerJeff King, Jun 9, 2014
  29. 14/15 commit: convert commit->buffer to a slabJeff King, Jun 9, 2014
  30. 15/15 commit: record buffer length in cacheJeff King, Jun 9, 2014
  31. Christian CouderJun 10, 2014
  32. Jeff KingJun 10, 2014
  33. Jeff KingJun 10, 2014
  34. Christian CouderJun 10, 2014
  35. 0/17 store length of commit->bufferJeff King, Jun 10, 2014
  36. 01/17 commit_tree: take a pointer/len pair rather than a const strbufJeff King, Jun 10, 2014
  37. 02/17 replace dangerous uses of strbuf_attachJeff King, Jun 10, 2014
  38. 03/17 alloc: include any-object allocations in alloc_reportJeff King, Jun 10, 2014
  39. 04/17 commit: push commit_index update into alloc_commit_nodeJeff King, Jun 10, 2014
  40. 05/17 do not create "struct commit" with xcallocJeff King, Jun 10, 2014
  41. 06/17 logmsg_reencode: return const bufferJeff King, Jun 10, 2014
  42. 07/17 sequencer: use logmsg_reencode in get_messageJeff King, Jun 10, 2014
  43. 08/17 provide a helper to free commit bufferJeff King, Jun 10, 2014
  44. Junio C HamanoJun 12, 2014
  45. Jeff KingJun 12, 2014
  46. Jeff KingJun 12, 2014
  47. 09/17 provide a helper to set the commit bufferJeff King, Jun 10, 2014
  48. 10/17 provide helpers to access the commit bufferJeff King, Jun 10, 2014
  49. 11/17 use get_cached_commit_buffer where appropriateJeff King, Jun 10, 2014
  50. 12/17 use get_commit_buffer to avoid duplicate codeJeff King, Jun 10, 2014
  51. 13/17 convert logmsg_reencode to get_commit_bufferJeff King, Jun 10, 2014
  52. 14/17 use get_commit_buffer everywhereJeff King, Jun 10, 2014
  53. 15/17 commit-slab: provide a static initializerJeff King, Jun 10, 2014
  54. Junio C HamanoJun 12, 2014
  55. Jeff KingJun 12, 2014
  56. 16/17 commit: convert commit->buffer to a slabJeff King, Jun 10, 2014
  57. 17/17 commit: record buffer length in cacheJeff King, Jun 10, 2014
  58. Jeff KingJun 10, 2014
  59. Junio C HamanoJun 12, 2014
  60. reuse cached commit buffer when parsing signaturesJeff King, Jun 13, 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.