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

Daniel Barkalow [off-list ref] writes:

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.

---

 connect.c   |    4 ++--
 transport.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/connect.c b/connect.c
index 06d279e..3d5c4ab 100644
--- a/connect.c
+++ b/connect.c

@@ -72,9 +72,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
 			continue;
 		if (nr_match && !path_match(name, nr_match, match))
 			continue;
-		ref = alloc_ref(len - 40);
+		ref = alloc_ref(name_len + 1);
 		hashcpy(ref->old_sha1, old_sha1);
-		memcpy(ref->name, buffer + 41, len - 40);
+		memcpy(ref->name, buffer + 41, name_len + 1);
 		*list = ref;
 		list = &ref->next;
 	}

diff --git a/transport.c b/transport.c
index 4f9cddc..3475cca 100644
--- a/transport.c
+++ b/transport.c

@@ -215,7 +215,7 @@ static struct ref *get_refs_from_bundle(const struct transport *transport)
 		die ("Could not read bundle '%s'.", transport->url);
 	for (i = 0; i < data->header.references.nr; i++) {
 		struct ref_list_entry *e = data->header.references.list + i;
-		struct ref *ref = alloc_ref(strlen(e->name));
+		struct ref *ref = alloc_ref(strlen(e->name) + 1);
 		hashcpy(ref->old_sha1, e->sha1);
 		strcpy(ref->name, e->name);
 		ref->next = result;