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

From

Junio C Hamano <gitster@pobox.com>

Date

Sep 28, 2007, 08:46 UTC

Message-ID

<7vhclfqisq.fsf@gitster.siamese.dyndns.org>

In-Reply-To

<Pine.LNX.4.64.0709280046241.5926@iabervon.org>

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.

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;