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.
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'
?
> @@ -2456,7 +2457,7 @@ static void file_change_deleteall(struct branch *b)
> static void parse_from_commit(struct branch *b, char *buf, unsigned long size)
> {
> - if (!buf || size < GIT_SHA1_HEXSZ + 6)
> + if (!buf || size < the_hash_algo->hexsz + 6)
...as it seems to have here...
> @@ -2555,7 +2556,7 @@ static struct hash_list *parse_merge(unsigned int *count)
> - if (!buf || size < 46)
> + if (!buf || size < the_hash_algo->hexsz)
...but not here.
