lockfile: change lock_file::filename into a strbuf

For now, we still make sure to allocate at least PATH_MAX characters
for the strbuf because resolve_symlink() doesn't know how to expand
the space for its return value.  (That will be fixed in a moment.)

Another alternative would be to just use a strbuf as scratch space in
lock_file() but then store a pointer to the naked string in struct
lock_file.  But lock_file objects are often reused.  By reusing the
same strbuf, we can avoid having to reallocate the string most times
when a lock_file object is reused.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index 70f5935f1b226e820babd054f81ab30e1732310b..f55e80986ba1787145690a8831baa632e6dec2fc 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -341,7 +341,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
                        die(_("unable to create temporary index"));
 
                old_index_env = getenv(INDEX_ENVIRONMENT);
-               setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
+               setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);
 
                if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
                        die(_("interactive add failed"));
@@ -352,7 +352,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
                        unsetenv(INDEX_ENVIRONMENT);
 
                discard_cache();
-               read_cache_from(index_lock.filename);
+               read_cache_from(index_lock.filename.buf);
                if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
                        if (reopen_lock_file(&index_lock) < 0)
                                die(_("unable to write index file"));
@@ -362,7 +362,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
                        warning(_("Failed to update main cache tree"));
 
                commit_style = COMMIT_NORMAL;
-               return index_lock.filename;
+               return index_lock.filename.buf;
        }
 
        /*
@@ -385,7 +385,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
                if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
                        die(_("unable to write new_index file"));
                commit_style = COMMIT_NORMAL;
-               return index_lock.filename;
+               return index_lock.filename.buf;
        }
 
        /*
@@ -472,9 +472,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
                die(_("unable to write temporary index file"));
 
        discard_cache();
-       read_cache_from(false_lock.filename);
+       read_cache_from(false_lock.filename.buf);
 
-       return false_lock.filename;
+       return false_lock.filename.buf;
 }
 
 static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,

diff --git a/builtin/reflog.c b/builtin/reflog.c
index e8a8fb13b9ffc8d3276048131e33e9874bc3bdea..7c78b15c149bb61d2e572fdfeda1c65bf08f8806 100644 (file)
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -431,7 +431,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
                         write_str_in_full(lock->lock_fd, "\n") != 1 ||
                         close_ref(lock) < 0)) {
                        status |= error("Couldn't write %s",
-                               lock->lk->filename);
+                                       lock->lk->filename.buf);
                        unlink(newlog_path);
                } else if (rename(newlog_path, log_file)) {
                        status |= error("cannot rename %s to %s",

diff --git a/cache.h b/cache.h
index c2ea6f15e6f5b7acf21abb95e7b1cd91a66473c6..f81d95fc3c6fa5c67a25a885e37fef2504e64f00 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -580,7 +580,7 @@ struct lock_file {
        volatile int fd;
        volatile pid_t owner;
        char on_list;
-       char filename[PATH_MAX];
+       struct strbuf filename;
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2

diff --git a/config.c b/config.c
index 123ed2996353dc5e6cba9e8b5548123eef296a45..21107797d31c8a92c923dc91bb9cf02db4ba75c4 100644 (file)
--- a/config.c
+++ b/config.c
@@ -2024,9 +2024,9 @@ int git_config_set_multivar_in_file(const char *config_filename,
                        MAP_PRIVATE, in_fd, 0);
                close(in_fd);
 
-               if (chmod(lock->filename, st.st_mode & 07777) < 0) {
+               if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {
                        error("chmod on %s failed: %s",
-                               lock->filename, strerror(errno));
+                               lock->filename.buf, strerror(errno));
                        ret = CONFIG_NO_WRITE;
                        goto out_free;
                }
@@ -2106,7 +2106,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
        return ret;
 
 write_err_out:
-       ret = write_error(lock->filename);
+       ret = write_error(lock->filename.buf);
        goto out_free;
 
 }
@@ -2207,9 +2207,9 @@ int git_config_rename_section_in_file(const char *config_filename,
 
        fstat(fileno(config_file), &st);
 
-       if (chmod(lock->filename, st.st_mode & 07777) < 0) {
+       if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {
                ret = error("chmod on %s failed: %s",
-                               lock->filename, strerror(errno));
+                               lock->filename.buf, strerror(errno));
                goto out;
        }
 
@@ -2230,7 +2230,7 @@ int git_config_rename_section_in_file(const char *config_filename,
                                }
                                store.baselen = strlen(new_name);
                                if (!store_write_section(out_fd, new_name)) {
-                                       ret = write_error(lock->filename);
+                                       ret = write_error(lock->filename.buf);
                                        goto out;
                                }
                                /*
@@ -2256,7 +2256,7 @@ int git_config_rename_section_in_file(const char *config_filename,
                        continue;
                length = strlen(output);
                if (write_in_full(out_fd, output, length) != length) {
-                       ret = write_error(lock->filename);
+                       ret = write_error(lock->filename.buf);
                        goto out;
                }
        }

diff --git a/lockfile.c b/lockfile.c
index 1dd118f5566b9f29c840e528b020383964abe87e..85c8648c51ebff7f47da1763e77ad1cddac32a4e 100644 (file)
--- a/lockfile.c
+++ b/lockfile.c
@@ -47,9 +47,9 @@
  *   failed attempt to lock, or a failed close_lock_file()).  In this
  *   state:
  *   - active is unset
- *   - filename[0] == '\0' (usually, though there are transitory states
- *     in which this condition doesn't hold). Client code should *not*
- *     rely on this fact!
+ *   - filename is empty (usually, though there are transitory
+ *     states in which this condition doesn't hold). Client code should
+ *     *not* rely on the filename being empty in this state.
  *   - fd is -1
  *   - the object is left registered in the lock_file_list, and
  *     on_list is set.
@@ -170,13 +170,6 @@ static char *resolve_symlink(char *p, size_t s)
 /* Make sure errno contains a meaningful value on error */
 static int lock_file(struct lock_file *lk, const char *path, int flags)
 {
-       /*
-        * subtract LOCK_SUFFIX_LEN from size to make sure there's
-        * room for adding ".lock" for the lock file name:
-        */
-       static const size_t max_path_len = sizeof(lk->filename) -
-                                          LOCK_SUFFIX_LEN;
-
        if (!lock_file_list) {
                /* One-time initialization */
                sigchain_push_common(remove_lock_file_on_signal);
@@ -191,30 +184,32 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
                lk->fd = -1;
                lk->active = 0;
                lk->owner = 0;
-               lk->filename[0] = 0;
+               strbuf_init(&lk->filename, PATH_MAX);
                lk->next = lock_file_list;
                lock_file_list = lk;
                lk->on_list = 1;
+       } else if (lk->filename.len) {
+               /* This shouldn't happen, but better safe than sorry. */
+               die("BUG: lock_file(\"%s\") called with improperly-reset lock_file object",
+                   path);
        }
 
-       if (strlen(path) >= max_path_len) {
-               errno = ENAMETOOLONG;
-               return -1;
+       strbuf_addstr(&lk->filename, path);
+       if (!(flags & LOCK_NODEREF)) {
+               resolve_symlink(lk->filename.buf, lk->filename.alloc);
+               strbuf_setlen(&lk->filename, strlen(lk->filename.buf));
        }
-       strcpy(lk->filename, path);
-       if (!(flags & LOCK_NODEREF))
-               resolve_symlink(lk->filename, max_path_len);
-       strcat(lk->filename, LOCK_SUFFIX);
-       lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
+       strbuf_addstr(&lk->filename, LOCK_SUFFIX);
+       lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
        if (lk->fd < 0) {
-               lk->filename[0] = 0;
+               strbuf_reset(&lk->filename);
                return -1;
        }
        lk->owner = getpid();
        lk->active = 1;
-       if (adjust_shared_perm(lk->filename)) {
+       if (adjust_shared_perm(lk->filename.buf)) {
                int save_errno = errno;
-               error("cannot fix permission bits on %s", lk->filename);
+               error("cannot fix permission bits on %s", lk->filename.buf);
                rollback_lock_file(lk);
                errno = save_errno;
                return -1;
@@ -313,7 +308,7 @@ int reopen_lock_file(struct lock_file *lk)
                die(_("BUG: reopen a lockfile that is still open"));
        if (!lk->active)
                die(_("BUG: reopen a lockfile that has been committed"));
-       lk->fd = open(lk->filename, O_WRONLY);
+       lk->fd = open(lk->filename.buf, O_WRONLY);
        return lk->fd;
 }
 
@@ -329,9 +324,9 @@ int commit_lock_file(struct lock_file *lk)
                return -1;
 
        /* remove ".lock": */
-       strbuf_add(&result_file, lk->filename,
-                  strlen(lk->filename) - LOCK_SUFFIX_LEN);
-       err = rename(lk->filename, result_file.buf);
+       strbuf_add(&result_file, lk->filename.buf,
+                  lk->filename.len - LOCK_SUFFIX_LEN);
+       err = rename(lk->filename.buf, result_file.buf);
        strbuf_reset(&result_file);
        if (err) {
                int save_errno = errno;
@@ -341,7 +336,7 @@ int commit_lock_file(struct lock_file *lk)
        }
 
        lk->active = 0;
-       lk->filename[0] = 0;
+       strbuf_reset(&lk->filename);
        return 0;
 }
 
@@ -359,8 +354,8 @@ void rollback_lock_file(struct lock_file *lk)
                return;
 
        if (!close_lock_file(lk)) {
-               unlink_or_warn(lk->filename);
+               unlink_or_warn(lk->filename.buf);
                lk->active = 0;
-               lk->filename[0] = 0;
+               strbuf_reset(&lk->filename);
        }
 }

diff --git a/read-cache.c b/read-cache.c
index af69f344a232ea8b1930d2ab273ff1782232e7bc..91bf876ee63e2c85078cae9c1eb7f8429dd6674c 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -2044,10 +2044,10 @@ static int commit_locked_index(struct lock_file *lk)
        if (alternate_index_output) {
                if (close_lock_file(lk))
                        return -1;
-               if (rename(lk->filename, alternate_index_output))
+               if (rename(lk->filename.buf, alternate_index_output))
                        return -1;
                lk->active = 0;
-               lk->filename[0] = 0;
+               strbuf_reset(&lk->filename);
                return 0;
        } else {
                return commit_lock_file(lk);

diff --git a/refs.c b/refs.c
index a4151315e32f38b89caa42704d16048b6c718521..598f4eb9e7555304d170db738c40af830d4a91de 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2607,8 +2607,8 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)
                 * lockfile name, minus ".lock":
                 */
                char *loose_filename = xmemdupz(
-                               lock->lk->filename,
-                               strlen(lock->lk->filename) - LOCK_SUFFIX_LEN);
+                               lock->lk->filename.buf,
+                               lock->lk->filename.len - LOCK_SUFFIX_LEN);
                int err = unlink_or_warn(loose_filename);
                free(loose_filename);
                if (err && errno != ENOENT)
@@ -2972,7 +2972,7 @@ int write_ref_sha1(struct ref_lock *lock,
            write_in_full(lock->lock_fd, &term, 1) != 1 ||
            close_ref(lock) < 0) {
                int save_errno = errno;
-               error("Couldn't write %s", lock->lk->filename);
+               error("Couldn't write %s", lock->lk->filename.buf);
                unlock_ref(lock);
                errno = save_errno;
                return -1;

diff --git a/shallow.c b/shallow.c
index 57f4afa6b40585c663d0df78494ce2108611ce4b..4919baf7727bd25f35d76de5f40e27f6725d6881 100644 (file)
--- a/shallow.c
+++ b/shallow.c
@@ -269,8 +269,8 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
        if (write_shallow_commits(&sb, 0, extra)) {
                if (write_in_full(fd, sb.buf, sb.len) != sb.len)
                        die_errno("failed to write to %s",
-                                 shallow_lock->filename);
-               *alternate_shallow_file = shallow_lock->filename;
+                                 shallow_lock->filename.buf);
+               *alternate_shallow_file = shallow_lock->filename.buf;
        } else
                /*
                 * is_repository_shallow() sees empty string as "no
@@ -316,7 +316,7 @@ void prune_shallow(int show_only)
        if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
                if (write_in_full(fd, sb.buf, sb.len) != sb.len)
                        die_errno("failed to write to %s",
-                                 shallow_lock.filename);
+                                 shallow_lock.filename.buf);
                commit_lock_file(&shallow_lock);
        } else {
                unlink(git_path("shallow"));