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>

Jeff King committed Sep 24, 2015 at 17:07 UTC d4b3d11a03c5733a37656ca2f23171be6efad7d3
1 file changed +22 -20
sha1_file.c
+22 -20
@@ -3011,29 +3011,31 @@ static inline int directory_size(const char *filename)
3011 * We want to avoid cross-directory filename renames, because those
3012 * can have problems on various filesystems (FAT, NFS, Coda).
3013 */
3014 -static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
3014 +static int create_tmpfile(struct strbuf *tmp, const char *filename)
3015 {
3016 int fd, dirlen = directory_size(filename);
3017
3018 - if (dirlen + 20 > bufsiz) {
3019 - errno = ENAMETOOLONG;
3020 - return -1;
3021 - }
3022 - memcpy(buffer, filename, dirlen);
3023 - strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
3024 - fd = git_mkstemp_mode(buffer, 0444);
3018 + strbuf_reset(tmp);
3019 + strbuf_add(tmp, filename, dirlen);
3020 + strbuf_addstr(tmp, "tmp_obj_XXXXXX");
3021 + fd = git_mkstemp_mode(tmp->buf, 0444);
3022 if (fd < 0 && dirlen && errno == ENOENT) {
3026 - /* Make sure the directory exists */
3027 - memcpy(buffer, filename, dirlen);
3028 - buffer[dirlen-1] = 0;
3029 - if (mkdir(buffer, 0777) && errno != EEXIST)
3023 + /*
3024 + * Make sure the directory exists; note that the contents
3025 + * of the buffer are undefined after mkstemp returns an
3026 + * error, so we have to rewrite the whole buffer from
3027 + * scratch.
3028 + */
3029 + strbuf_reset(tmp);
3030 + strbuf_add(tmp, filename, dirlen - 1);
3031 + if (mkdir(tmp->buf, 0777) && errno != EEXIST)
3032 return -1;
3031 - if (adjust_shared_perm(buffer))
3033 + if (adjust_shared_perm(tmp->buf))
3034 return -1;
3035
3036 /* Try again */
3035 - strcpy(buffer + dirlen - 1, "/tmp_obj_XXXXXX");
3036 - fd = git_mkstemp_mode(buffer, 0444);
3037 + strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
3038 + fd = git_mkstemp_mode(tmp->buf, 0444);
3039 }
3040 return fd;
3041 }
@@ -3046,10 +3048,10 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
3048 git_zstream stream;
3049 git_SHA_CTX c;
3050 unsigned char parano_sha1[20];
3049 - static char tmp_file[PATH_MAX];
3051 + static struct strbuf tmp_file = STRBUF_INIT;
3052 const char *filename = sha1_file_name(sha1);
3053
3052 - fd = create_tmpfile(tmp_file, sizeof(tmp_file), filename);
3054 + fd = create_tmpfile(&tmp_file, filename);
3055 if (fd < 0) {
3056 if (errno == EACCES)
3057 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,
3100 struct utimbuf utb;
3101 utb.actime = mtime;
3102 utb.modtime = mtime;
3101 - if (utime(tmp_file, &utb) < 0)
3103 + if (utime(tmp_file.buf, &utb) < 0)
3104 warning("failed utime() on %s: %s",
3103 - tmp_file, strerror(errno));
3105 + tmp_file.buf, strerror(errno));
3106 }
3107
3106 - return finalize_object_file(tmp_file, filename);
3108 + return finalize_object_file(tmp_file.buf, filename);
3109 }
3110
3111 static int freshen_loose_object(const unsigned char *sha1)