git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Pierre Habouzit <madcoder@debian.org>
Date
Sep 28, 2007, 12:41 UTC
Message-ID
<20070928124102.GA21309@artemis.corp>
In-Reply-To
<Pine.LNX.4.64.0709281259050.28395@racer.site>
On Fri, Sep 28, 2007 at 12:01:28PM +0000, Johannes Schindelin wrote:
Show 9 quoted lines
> 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
Previous: Johannes SchindelinNext: Pierre Habouzit
Message 5 of 9 in “alloc_ref(): allow for trailing NUL”
  1. alloc_ref(): allow for trailing NULJohannes Schindelin, Sep 28, 2007
  2. Daniel BarkalowSep 28, 2007
  3. Junio C HamanoSep 28, 2007
  4. Johannes SchindelinSep 28, 2007
  5. Pierre HabouzitSep 28, 2007
  6. Pierre HabouzitSep 28, 2007
  7. Daniel BarkalowSep 28, 2007
  8. Johannes SchindelinSep 28, 2007
  9. Junio C HamanoSep 28, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.