Hi, On Fri, 28 Sep 2007, Junio C Hamano wrote: > Daniel Barkalow <barkalow@xxxxxxxxxxxx> 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 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
