Re: [PATCH 12/31] fast-import: make hash-size independent

> On Mon, Feb 11, 2019 at 8:23 PM brian m. carlson
> <sandals@crustytoothpaste.net> 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@crustytoothpaste.net>
> > ---
> > 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.

> 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'?

> > @@ -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'
> 
> ?