Patryk Obara [off-list ref] writes:
The previous implementation of read_graft_line used calculations based
on GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ to determine the number of commit
ids in a single graft line. New implementation does not depend on these
constants, so it adapts to any object_id buffer size.
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[].
Is this because you expect more than one _kind_ of hashes are used
at the same time?
Junio C Hamano [off-list ref] wrote:
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:
- 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).
Is this because you expect more than one _kind_ of hashes are used
at the same time?
Yes - I think more than one kind of hashes will be used at the same time
(meaning in single git binary, not necessarily in a single process), at
the very least to allow read access to repositories in "old" format(s), with
sha1-based grafts. Using MAX for parsing here would prevent this,
requiring either modifying grafts code again in future or forcing right now
some unwelcome design decisions down the line.
My goal here was to make this code compatible with whatever hash
function transition plan will be chosen.
--
| ← Ceci n'est pas une pipe
Patryk Obara