Hi Junio,
On 2015-06-26 19:37, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
On Fri, Jun 26, 2015 at 10:06:20AM +0200, Johannes Schindelin wrote:
quoted
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