Re: [PATCH 5/5] commit: rewrite read_graft_line

Patryk Obara [off-list ref] writes:

Junio C Hamano [off-list ref] wrote:

quoted

I am not sure if this is a good approach.  Just like in 2/5 you can
use the MAX thing instead of 20, instead of having each graft entry
allocate a separate oid_array.oid[].

Once MAX values were increased memory corruption was caused exactly by
this line:

quoted

- graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));

I could've replaced it by:

    graft = xmalloc(st_add(sizeof(*graft), st_mult(sizeof(struct
object_id), i)));

But it seemed to me like short-sighted solution (code might be broken if
object_id will be modified again in future).

Why?  

Assuming that "struct object_id" would have to be prepared to handle
more than one types of hash functions at the same time, it would be
sufficiently large to hold any type of hash, plus possibly another
member that tells which kind.

The decision to choose between a flex array and a separately
allocatable structure primarily should come from how the enclosing
struct (i.e. the commti_graft structure) is meant to be used.  It is
not meant to be tweaked by adding more parents or removing parents
once it is constructed, which argues for having a flex array to hold
the known and fixed number of parents once it is constructed.