Re: [PATCH 56/67] 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 <redacted>
---

diff --git a/archive.c b/archive.c
index 01b0899..4ac86c8 100644
--- a/archive.c
+++ b/archive.c

@@ -171,13 +171,14 @@ static void queue_directory(const unsigned char *sha1,
                unsigned mode, int stage, struct archiver_context *c)
 {
        struct directory *d;
-       d = xmallocz(sizeof(*d) + base->len + 1 + strlen(filename));
+       size_t len = base->len + 1 + strlen(filename) + 1;
+       d = xmalloc(sizeof(*d) + len);

        hashcpy(d->oid.hash, sha1);
 }

diff --git a/fast-import.c b/fast-import.c
index d0c2502..895c6b4 100644
--- a/fast-import.c
+++ b/fast-import.c

@@ -863,13 +863,15 @@ static void start_packfile(void)
 {
        static char tmp_file[PATH_MAX];
        struct packed_git *p;
+       int namelen;
        struct pack_header hdr;
        int pack_fd;

        pack_fd = odb_mkstemp(tmp_file, sizeof(tmp_file),
                              "pack/tmp_pack_XXXXXX");
-       p = xcalloc(1, sizeof(*p) + strlen(tmp_file) + 2);
-       strcpy(p->pack_name, tmp_file);
+       namelen = strlen(tmp_file) + 2;

+       p = xcalloc(1, sizeof(*p) + namelen);
+       xsnprintf(p->pack_name, namelen, "%s", tmp_file);
        p->pack_fd = pack_fd;
        p->do_not_close = 1;
        pack_file = sha1fd(pack_fd, p->pack_name);

diff --git a/refs.c b/refs.c
index c2709de..df6c41a 100644
--- a/refs.c
+++ b/refs.c

@@ -3984,10 +3984,10 @@ void ref_transaction_free(struct ref_transaction *transaction)
 static struct ref_update *add_update(struct ref_transaction *transaction,
                                     const char *refname)
 {
-       size_t len = strlen(refname);
-       struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
+       size_t len = strlen(refname) + 1;
+       struct ref_update *update = xcalloc(1, sizeof(*update) + len);

-       strcpy((char *)update->refname, refname);
+       memcpy((char *)update->refname, refname, len); /* includese NUL */