On Fri, Sep 28, 2007 at 12:01:28PM +0000, Johannes Schindelin wrote:
> 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.
<janitor cap on>
While being at it, I noticed that the following code pattern is used
in many places in git:
struct something *ptr = xmalloc(sizeof(struct something) + some_string_len + 1);
memset(ptr, 0, sizeof(struct something));
memcpy(ptr->name, somestring, some_string_len + 1);
With name being a flex array.
Maybe we could deal with all of these, and make a generic construction
that would take care of allocating the proper space
I believe we could have function doing that, that would factorize this
code, and use a nice api à la xmemdupz for this.
It would be something like:
void *xflexdupz(size_t offset, void *src, size_t len)
{
char *p = xmalloc(offset + len + 1);
memset(p, 0, offset);
memcpy(p + offset, src, len);
p[offset + len] = '\0';
return p;
}
Then alloc_ref could be a wrapper around:
xflexdupz(offsetof(struct ref, name), ..., ...).
Of course right now alloc_ref doesn't perform any kind of copy, but a
grep -A1 will convince you that it's not a problem:
$ grep -A1 alloc_ref **/*.[hc]
builtin-fetch.c: rm = alloc_ref(strlen(ref_name) + 1);
builtin-fetch.c- strcpy(rm->name, ref_name);
builtin-fetch.c: rm->peer_ref = alloc_ref(strlen(ref_name) + 1);
builtin-fetch.c- strcpy(rm->peer_ref->name, ref_name);
--
connect.c: ref = alloc_ref(len - 40);
connect.c- hashcpy(ref->old_sha1, old_sha1);
--
remote.c:struct ref *alloc_ref(unsigned namelen)
remote.c-{
--
remote.c: ref = alloc_ref(20);
remote.c- strcpy(ref->name, "(delete)");
--
remote.c: ref = alloc_ref(len);
remote.c- memcpy(ref->name, name, len);
--
remote.c: ret = alloc_ref(len);
remote.c- memcpy(ret->name, name, len);
--
remote.c: cpy->peer_ref = alloc_ref(local_prefix_len +
remote.c- strlen(match) + 1);
--
remote.c: ret = alloc_ref(strlen(name) + 1);
remote.c- strcpy(ret->name, name);
--
remote.c: ret = alloc_ref(strlen(name) + 6);
remote.c- sprintf(ret->name, "refs/%s", name);
--
remote.c: ret = alloc_ref(strlen(name) + 12);
remote.c- sprintf(ret->name, "refs/heads/%s", name);
--
remote.h:struct ref *alloc_ref(unsigned namelen);
remote.h-
--
transport.c: struct ref *ref = alloc_ref(strlen(e->name));
transport.c- hashcpy(ref->old_sha1, e->sha1);
</janitor>
--
·O· Pierre Habouzit
··O madcoder@xxxxxxxxxx
OOO http://www.madism.org
Attachment:
pgpAaVe6ln2VZ.pgp
Description: PGP signature
