On Sat, May 10, 2008 at 04:26:58PM -0700, kkowalczyk@xxxxxxxxx wrote:
> As a byproduct, fixes one place where string wasn't properly terminated.
Great. Does this fix a user-visible bug? It would be nice to mention in
the commit log _which_ place (though after reading the patch carefully,
it looks like the one interpret_target) so that people looking at the
commit later can understand exactly what was fixed.
> - ref = alloc_ref(strlen(refname) + 1);
> - strcpy(ref->name, refname);
> + ref = alloc_ref_from_str(refname);
So this turns a 2-line construct into a 1-line construct...
> +struct ref *alloc_ref_from_str(const char* str)
> +{
> + struct ref *ret;
> + unsigned len = strlen(str) + 1;
> + char *tmp = xmalloc(sizeof(struct ref) + len);
> + ret = (struct ref*)tmp;
> + memset(tmp, 0, sizeof(struct ref));
> + tmp += sizeof(struct ref);
> + memcpy(tmp, str, len);
> + return ret;
> +}
But why do we need an 8-line function to do it?
The only difference I can see over
struct ref *alloc_ref_from_str(const char *str)
{
unsigned len = strlen(str) + 1;
struct ref *ret = alloc_ref(len);
memcpy(ret->name, str, len);
return ret;
}
is that we avoid memsetting the name portion of the struct to 0 before
copying to it. It seems like an unproven micro-optimization that makes
it a bit harder to read.
-Peff
--
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
