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

Re: [PATCH 3/5] commit: replace the raw buffer with strbuf in read_graft_line

From
Patryk Obara <patryk.obara@gmail.com>
Date
Aug 17, 2017, 21:38 UTC
Message-ID
<CAJfL8+T0sC9TnYfdut1nqiE9e2misnjK6X0WFDkj-mPcuKB4Tw@mail.gmail.com>
In-Reply-To
<xmqq7ey1gai3.fsf@gitster.mtv.corp.google.com>
Show 5 quoted lines
> In fact, the former is already how we represent the list of fake
> parents in the commit_graft structure, so I think patch 5/5 in this
> series does two unrelated things, one of which is bad (i.e. use of
> parse_oid_hex() is good; turning the FLEX_ARRAY at the end into a
> oid_array that requires a separate allocation of the array is bad).

Agreed; I already split patch 5 into two separate changes (one fixing memory allocation issue, one parsing object_ids into FLEX_ARRAY, without modifying graft struct). In result patch 4 (free_graft) can be dropped. I will send these changes as v3.

On Thu, Aug 17, 2017 at 11:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> I'd expect most of the GIT_MAX constants to eventually go away in favor
>> of "struct object_id", but that will still be using the same "big enough
>> to hold any hash" size under the hood.
>
> Indeed.  It is good to see major contributors are in agreement ;-)
> I'd expect that an array of "struct object_id" would be how a fixed
> number of object names would be represented, i.e.
>
>         struct object_id thing[num_elements];
>
> instead of an array of uchar that is MAX bytes long, i.e.
>
>         unsigned char name[GIT_MAX_RAWSZ][num_elements];
>
> In fact, the former is already how we represent the list of fake
> parents in the commit_graft structure, so I think patch 5/5 in this
> series does two unrelated things, one of which is bad (i.e. use of
> parse_oid_hex() is good; turning the FLEX_ARRAY at the end into a
> oid_array that requires a separate allocation of the array is bad).
>
>> Agreed. Most code should be dealing with the abstract concept of a hash
>> and shouldn't have to care about the size. I really like parse_oid_hex()
>> for that reason (and I think parsing is the main place we've found that
>> needs to care).
>
> Yes.
-- 
| ← Ceci n'est pas une pipe
Patryk Obara
Previous: Junio C HamanoNext: Junio C Hamano
Message 19 of 59 in “Modernize read_graft_line implementation”
  1. 0/5 Modernize read_graft_line implementationPatryk Obara, Aug 15, 2017
  2. 1/5 cache: extend object_id size to sha3-256Patryk Obara, Aug 15, 2017
  3. 2/5 sha1_file: fix hardcoded size in null_sha1Patryk Obara, Aug 15, 2017
  4. Junio C HamanoAug 15, 2017
  5. Stefan BellerAug 15, 2017
  6. Junio C HamanoAug 15, 2017
  7. Patryk ObaraAug 16, 2017
  8. Junio C HamanoAug 16, 2017
  9. 4/5 commit: implement free_commit_graftPatryk Obara, Aug 15, 2017
  10. Stefan BellerAug 15, 2017
  11. Junio C HamanoAug 15, 2017
  12. Patryk ObaraAug 16, 2017
  13. 3/5 commit: replace the raw buffer with strbuf in read_graft_linePatryk Obara, Aug 15, 2017
  14. Stefan BellerAug 15, 2017
  15. Patryk ObaraAug 16, 2017
  16. brian m. carlsonAug 16, 2017
  17. Jeff KingAug 17, 2017
  18. Junio C HamanoAug 17, 2017
  19. Patryk ObaraAug 17, 2017
  20. Junio C HamanoAug 15, 2017
  21. 5/5 commit: rewrite read_graft_linePatryk Obara, Aug 15, 2017
  22. Stefan BellerAug 15, 2017
  23. Junio C HamanoAug 15, 2017
  24. Patryk ObaraAug 16, 2017
  25. Junio C HamanoAug 16, 2017
  26. Stefan BellerAug 15, 2017
  27. 0/4 Modernize read_graft_line implementationPatryk Obara, Aug 16, 2017
  28. 1/4 sha1_file: fix hardcoded size in null_sha1Patryk Obara, Aug 16, 2017
  29. Junio C HamanoAug 16, 2017
  30. 3/4 commit: implement free_commit_graftPatryk Obara, Aug 16, 2017
  31. 2/4 commit: replace the raw buffer with strbuf in read_graft_linePatryk Obara, Aug 16, 2017
  32. 4/4 commit: rewrite read_graft_linePatryk Obara, Aug 16, 2017
  33. Junio C HamanoAug 17, 2017
  34. Patryk ObaraAug 17, 2017
  35. 0/4 Modernize read_graft_line implementationPatryk Obara, Aug 18, 2017
  36. 2/4 commit: replace the raw buffer with strbuf in read_graft_linePatryk Obara, Aug 18, 2017
  37. Jeff KingAug 18, 2017
  38. Patryk ObaraAug 18, 2017
  39. Jeff KingAug 18, 2017
  40. 1/4 sha1_file: fix definition of null_sha1Patryk Obara, Aug 18, 2017
  41. 4/4 commit: rewrite read_graft_linePatryk Obara, Aug 18, 2017
  42. Jeff KingAug 18, 2017
  43. Junio C HamanoAug 18, 2017
  44. Patryk ObaraAug 18, 2017
  45. Jeff KingAug 18, 2017
  46. Junio C HamanoAug 18, 2017
  47. Patryk ObaraAug 18, 2017
  48. Junio C HamanoAug 18, 2017
  49. 3/4 commit: allocate array using object_id sizePatryk Obara, Aug 18, 2017
  50. Junio C HamanoAug 18, 2017
  51. 0/4 Modernize read_graft_line implementationPatryk Obara, Aug 18, 2017
  52. 2/4 commit: replace the raw buffer with strbuf in read_graft_linePatryk Obara, Aug 18, 2017
  53. 4/4 commit: rewrite read_graft_linePatryk Obara, Aug 18, 2017
  54. Patryk ObaraAug 18, 2017
  55. Junio C HamanoAug 18, 2017
  56. Patryk ObaraAug 18, 2017
  57. Junio C HamanoAug 18, 2017
  58. 1/4 sha1_file: fix definition of null_sha1Patryk Obara, Aug 18, 2017
  59. 3/4 commit: allocate array using object_id sizePatryk Obara, Aug 18, 2017

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.