On Mon, Feb 11, 2019 at 10:44:12PM -0500, Eric Sunshine wrote: > On Mon, Feb 11, 2019 at 8:23 PM brian m. carlson > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > Replace several uses of GIT_SHA1_HEXSZ and 40-based constants with > > references to the_hash_algo. Update the note handling code here to > > compute path sizes based on GIT_MAX_RAWSZ as well. > > > > Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> > > --- > > diff --git a/fast-import.c b/fast-import.c > > @@ -2047,7 +2047,8 @@ static uintmax_t do_change_note_fanout( > > - char realpath[60]; > > + char realpath[GIT_MAX_RAWSZ * 3]; > > I wonder if the fixed multiplier deserves a comment explaining that > this is reserving enough space for a hex representation with '/' > between each digit pair plus NUL. Which leads to the next question: Is > there is GIT_MAX_HEXSZ constant? If so, this might be more clearly > represented (or not) by taking advantage of that value. There is such a constant. I'll add a comment and see if I can write it in a way that makes it more intuitive what we're computing. > Also, there are a number of hardcoded 60's in the code earlier in this > file, such as: > > if ((max_packsize && (pack_size + 60 + len) > max_packsize) > || (pack_size + 60 + len) < pack_size) > cycle_packfile(); > > Is that just a coincidence or is it related to the 60 characters > allocated for 'realpath'? That's an interesting question. It looks like these are indeed related to the hash size, but they're not related to the realpath call above. I'll convert them as well. > > @@ -2405,7 +2406,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa > > char *buf = read_object_with_reference(&commit_oid, > > commit_type, &size, > > &commit_oid); > > - if (!buf || size < 46) > > + if (!buf || size < the_hash_algo->hexsz) > > What exactly did the 46 represent and how does it relate to 'hexsz'? > Stated differently, why didn't this become: > > the_hash_algo->hexsz + 6' > > ? Good catch. I believe that 46 is the number of bytes in "tree %s\n", which is the smallest possible commit (a root commit without an author or committer). I'll fix this misconversion. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
