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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Fri, 28 Sep 2007, Johannes Schindelin wrote:
> >
> >> 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