From c7ab0ba3405dc6bc8ade1296ef070a5a89660e76 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Sep 2015 17:08:12 -0400 Subject: avoid sprintf and strcpy with flex arrays When we are allocating a struct with a FLEX_ARRAY member, we generally compute the size of the array and then sprintf or strcpy into it. Normally we could improve a dynamic allocation like this by using xstrfmt, but it doesn't work here; we have to account for the size of the rest of the struct. But we can improve things a bit by storing the length that we use for the allocation, and then feeding it to xsnprintf or memcpy, which makes it more obvious that we are not writing more than the allocated number of bytes. It would be nice if we had some kind of helper for allocating generic flex arrays, but it doesn't work that well: - the call signature is a little bit unwieldy: d = flex_struct(sizeof(*d), offsetof(d, path), fmt, ...); You need offsetof here instead of just writing to the end of the base size, because we don't know how the struct is packed (partially this is because FLEX_ARRAY might not be zero, though we can account for that; but the size of the struct may actually be rounded up for alignment, and we can't know that). - some sites do clever things, like over-allocating because they know they will write larger things into the buffer later (e.g., struct packed_git here). So we're better off to just write out each allocation (or add type-specific helpers, though many of these are one-off allocations anyway). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- submodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index 245ed4dfb..c480ed53b 100644 --- a/submodule.c +++ b/submodule.c @@ -122,6 +122,7 @@ static int add_submodule_odb(const char *path) struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; int ret = 0; + int alloc; const char *git_dir; strbuf_addf(&objects_directory, "%s/.git", path); @@ -142,9 +143,10 @@ static int add_submodule_odb(const char *path) objects_directory.len)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alloc = objects_directory.len + 42; /* for "12/345..." sha1 */ + alt_odb = xmalloc(sizeof(*alt_odb) + alloc); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); + xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf); alt_odb->name = alt_odb->base + objects_directory.len; alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; -- cgit v1.2.1