Re: git error in tag ...: unterminated header

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 27, 2015, 08:57 UTC
Message-ID: <5bf3e78138813d8667f4761cc7bc23a3@www.dscho.org>
In-Reply-To: <xmqqh9puv0a8.fsf@gitster.dls.corp.google.com>

Hi Junio,

On 2015-06-26 19:37, Junio C Hamano wrote:

Show 27 quoted lines

> Jeff King <peff@peff.net> writes:
> 
>> On Fri, Jun 26, 2015 at 10:06:20AM +0200, Johannes Schindelin wrote:
>>
>>> I understood what you were saying, but it still appears too fragile to
>>> me to mix functions that assume NUL-terminated strings with an ad-hoc
>>> counted string check.
>>
>> Yeah, I agree. It is not that you cannot make it safe, but that it is
>> simply a fragile maintenance burden in the future. I thought we dealt
>> with this already with a1e920a (index-pack: terminate object buffers
>> with NUL, 2014-12-08), though.
> 
> Hmph, that is an interesting point.
> 
> It would mean that the require_eoh() can be reduced a bit further.
> 
>  * It is still a good idea to make sure we do not have NUL in the
>    header part,
> 
>  * It can still stop scanning when it finds a blank line (i.e. we do
>    not care what is in the message part of commit and tag),
> 
>  * It does not have to insist that a commit or a tag has a blank
>    line to reject a header-only object.
> 
> That would mean the name of the helper needs to change, though.

You mean in addition to your changes to read new lines only when we're still inside the buffer? I cannot say that I like this fragility (and would prefer the aforementioned patch that simply allocates a NUL-terminated buffer in the rather unlikely event of tag/commit objects without an empty line), but then: you are stuck with maintaining this code, so it is your decision. ;-)

I will hopefully have time starting Tuesday this week to work on that patch, if nobody else beats me to it.

Ciao, Dscho