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