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

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@debian.org
OOO                                                http://www.madism.org