Re: [PATCH] fetch-pack: fix object_id of exact sha1

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 28, 2016, 19:29 UTC
Message-ID: <xmqqh9gseiqk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CABaesJJCfd6FzgEVCWMJH8-gKiv3pT8BGq4f-qFgprVde+ViQQ@mail.gmail.com>

Gabriel Souza Franco <gabrielfrancosouza@gmail.com> writes:

Show 23 quoted lines

>>>       struct object_id oid;
>>>
>>> -     if (!get_oid_hex(name, &oid) && name[GIT_SHA1_HEXSZ] == ' ')
>>> -             name += GIT_SHA1_HEXSZ + 1;
>>> -     else
>>> +     if (get_oid_hex(name, &oid))
>>>               oidclr(&oid);
>>> +     else if (name[GIT_SHA1_HEXSZ] == ' ')
>>> +             name += GIT_SHA1_HEXSZ + 1;
>>
>> This makes sense to me. I wonder if we should be more particular about
>> the pure-sha1 case consuming the whole string, though. E.g., if we get:
>>
>>   1234567890123456789012345678901234567890-bananas
>>
>> that should probably not have sha1 1234...
>>
>> I don't think it should ever really happen in practice, but it might be
>> worth noticing and complaining when name[GIT_SHA1_HEXSZ] is neither
>> space nor '\0'.
>
> Right. What kind of complaining? Is doing oidclr(&oid) and letting it
> fail elsewhere enough?

I think that is the most sensible. After all, the first get-oid-hex expects to find a valid 40-hex object name at the beginning of line, and oidclr() is the way for it to signal a broken input, which is how the callers of this codepath expects errors to be caught.

Thanks.