Re: [PATCH] alloc_ref(): allow for trailing NUL

Hi,

On Fri, 28 Sep 2007, Junio C Hamano wrote:

Daniel Barkalow [off-list ref] writes:

quoted

On Fri, 28 Sep 2007, Johannes Schindelin wrote:

quoted

The parameter name "namelen" suggests that you pass the equivalent of
strlen() to the function alloc_ref().  However, this function did not
allocate enough space to put a NUL after the name.

Since struct ref does not have any member to describe the length of the
string, this just does not make sense.

So make space for the NUL.

Good point, but shouldn't you then fix call sites that use strlen(name) + 
1?

Good point.

I audited "git grep -A2 -B4 -e alloc_ref next master" output,
and it appears almost everybody knows alloc_ref() wants the
caller to count the terminating NUL.

There however are a few gotchas.

 * There is one overallocation in connect.c, which would not
   hurt but is wasteful;

 * next:transport.c has alloc_ref(strlen(e->name)) which is a
   no-no;

Discarding Johannes's patch, the following would fix it.

But should the signature of alloc_ref() not be changed, then, to read

	struct ref *alloc_ref(unsigned name_alloc);

Hm?

Further, I am quite sure that the same mistake will happen again, until we 
change the function to get the name length, not the number of bytes to 
allocate.

Ciao,
Dscho