Jeff King [off-list ref] 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.
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 [off-list ref] wrote:
Jeff King [off-list ref] writes:
quoted
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).
quoted
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