write_loose_object: convert to strbuf

When creating a loose object tempfile, we use a fixed
PATH_MAX-sized buffer, and strcpy directly into it. This
isn't buggy, because we do a rough check of the size, but
there's no verification that our guesstimate of the required
space is enough (in fact, it's several bytes too big for the
current naming scheme).

Let's switch to a strbuf, which makes this much easier to
verify. The allocation overhead should be negligible, since
we are replacing a static buffer with a static strbuf, and
we'll only need to allocate on the first call.

While we're here, we can also document a subtle interaction
with mkstemp that would be easy to overlook.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_file.c b/sha1_file.c
index c26fdcbd8840ede1865210a09ee6c6f85794985b..4211af1d89be432bf0a232557f0b6874a5f271f0 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3011,29 +3011,31 @@ static inline int directory_size(const char *filename)
  * We want to avoid cross-directory filename renames, because those
  * can have problems on various filesystems (FAT, NFS, Coda).
  */
  * We want to avoid cross-directory filename renames, because those
  * can have problems on various filesystems (FAT, NFS, Coda).
  */

-static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
+static int create_tmpfile(struct strbuf *tmp, const char *filename)

 {
        int fd, dirlen = directory_size(filename);
 
 {
        int fd, dirlen = directory_size(filename);
 

-       if (dirlen + 20 > bufsiz) {
-               errno = ENAMETOOLONG;
-               return -1;
-       }
-       memcpy(buffer, filename, dirlen);
-       strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
-       fd = git_mkstemp_mode(buffer, 0444);
+       strbuf_reset(tmp);
+       strbuf_add(tmp, filename, dirlen);
+       strbuf_addstr(tmp, "tmp_obj_XXXXXX");
+       fd = git_mkstemp_mode(tmp->buf, 0444);

        if (fd < 0 && dirlen && errno == ENOENT) {
        if (fd < 0 && dirlen && errno == ENOENT) {

-               /* Make sure the directory exists */
-               memcpy(buffer, filename, dirlen);
-               buffer[dirlen-1] = 0;
-               if (mkdir(buffer, 0777) && errno != EEXIST)
+               /*
+                * Make sure the directory exists; note that the contents
+                * of the buffer are undefined after mkstemp returns an
+                * error, so we have to rewrite the whole buffer from
+                * scratch.
+                */
+               strbuf_reset(tmp);
+               strbuf_add(tmp, filename, dirlen - 1);
+               if (mkdir(tmp->buf, 0777) && errno != EEXIST)

                        return -1;
                        return -1;

-               if (adjust_shared_perm(buffer))
+               if (adjust_shared_perm(tmp->buf))

                        return -1;
 
                /* Try again */
                        return -1;
 
                /* Try again */

-               strcpy(buffer + dirlen - 1, "/tmp_obj_XXXXXX");
-               fd = git_mkstemp_mode(buffer, 0444);
+               strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
+               fd = git_mkstemp_mode(tmp->buf, 0444);

        }
        return fd;
 }
        }
        return fd;
 }


@@ -3046,10 +3048,10 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
        git_zstream stream;
        git_SHA_CTX c;
        unsigned char parano_sha1[20];
        git_zstream stream;
        git_SHA_CTX c;
        unsigned char parano_sha1[20];

-       static char tmp_file[PATH_MAX];
+       static struct strbuf tmp_file = STRBUF_INIT;

        const char *filename = sha1_file_name(sha1);
 
        const char *filename = sha1_file_name(sha1);
 

-       fd = create_tmpfile(tmp_file, sizeof(tmp_file), filename);
+       fd = create_tmpfile(&tmp_file, filename);

        if (fd < 0) {
                if (errno == EACCES)
                        return error("insufficient permission for adding an object to repository database %s", get_object_directory());
        if (fd < 0) {
                if (errno == EACCES)
                        return error("insufficient permission for adding an object to repository database %s", get_object_directory());


@@ -3098,12 +3100,12 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
                struct utimbuf utb;
                utb.actime = mtime;
                utb.modtime = mtime;
                struct utimbuf utb;
                utb.actime = mtime;
                utb.modtime = mtime;

-               if (utime(tmp_file, &utb) < 0)
+               if (utime(tmp_file.buf, &utb) < 0)

                        warning("failed utime() on %s: %s",
                        warning("failed utime() on %s: %s",

-                               tmp_file, strerror(errno));
+                               tmp_file.buf, strerror(errno));

        }
 
        }
 

-       return finalize_object_file(tmp_file, filename);
+       return finalize_object_file(tmp_file.buf, filename);

 }
 
 static int freshen_loose_object(const unsigned char *sha1)
 }
 
 static int freshen_loose_object(const unsigned char *sha1)