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