Re: [PATCH 2/5] sha1_file: fix hardcoded size in null_sha1
- From
Stefan Beller <sbeller@google.com>
- Date
- Aug 15, 2017, 18:29 UTC
- Message-ID
- <CAGZ79kaCA29j6ON4KSsB=EH8FPfZGE56hVGSAAepcPiH952v6g@mail.gmail.com>
- In-Reply-To
- <xmqq1socsnay.fsf@gitster.mtv.corp.google.com>
On Tue, Aug 15, 2017 at 11:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> Patryk Obara <patryk.obara@gmail.com> writes:
>
>> This prevents compilation error if GIT_MAX_RAWSZ is different than 20.
>>
>> Signed-off-by: Patryk Obara <patryk.obara@gmail.com>
>> ---
>> sha1_file.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> I think this is OK for "null" thing, but in general I feel
> ambivalent when I see the use of "MAX" thing.
>
> Use of "MAX" here implies that we wish to support multiple hashes at
> the same time in a single binary, so that longer or shorter hashes
> fit there. But in such a case, the callers would want a union of
> some sort so that they can say "I am using SHA2, give me the null
> thing"?
>
> I said this is OK for "null" because we assume we will use ^\0{len}$
> for any hash function we choose as the "impossible" value, and for
> that particular use pattern, we do not need such a union. Just
> letting the caller peek at an appropriate number of bytes at the
> beginning of that NUL buffer for hash the caller wants to use is
> sufficient.
>
> Thanks.Once we have 2 hash functions usable in a local Git installation, this would be wasteful for the smaller hash function (and the related grafts).
I think Jonathan once envisioned an 'optimized' version as a second step, maybe this is a good time to discuss how we'd get the right size for e.g. allocating memory, as _MAX_ seems to be not the correct solution long term?