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

> 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.