From: Michael Haggerty <mhagger@alum.mit.edu>
To: "Junio C Hamano" <gitster@pobox.com>,
"Johannes Sixt" <j6t@kdbg.org>,
"Torsten Bögershausen" <tboegi@web.de>
Cc: Jeff King <peff@peff.net>,
git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v4 28/32] Change lock_file::filename into a strbuf
Date: Sat, 6 Sep 2014 09:50:42 +0200 [thread overview]
Message-ID: <1409989846-22401-29-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1409989846-22401-1-git-send-email-mhagger@alum.mit.edu>
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>
---
builtin/commit.c | 12 ++++++------
builtin/reflog.c | 2 +-
cache.h | 2 +-
config.c | 14 +++++++-------
lockfile.c | 47 +++++++++++++++++++----------------------------
read-cache.c | 4 ++--
refs.c | 6 +++---
shallow.c | 6 +++---
8 files changed, 42 insertions(+), 51 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 046be7e..b2fc9e8 100644
--- 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,10 +352,10 @@ 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);
commit_style = COMMIT_NORMAL;
- return index_lock.filename;
+ return index_lock.filename.buf;
}
/*
@@ -378,7 +378,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;
}
/*
@@ -460,9 +460,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 e8a8fb1..7c78b15 100644
--- 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 f7ad1b5..bea6296 100644
--- a/cache.h
+++ b/cache.h
@@ -579,7 +579,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 24f0abb..d9247c6 100644
--- a/config.c
+++ b/config.c
@@ -1905,9 +1905,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;
}
@@ -1986,7 +1986,7 @@ out_free:
return ret;
write_err_out:
- ret = write_error(lock->filename);
+ ret = write_error(lock->filename.buf);
goto out_free;
}
@@ -2087,9 +2087,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;
}
@@ -2110,7 +2110,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;
}
/*
@@ -2136,7 +2136,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 f6b3866..c34bf6c 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -56,8 +56,8 @@
* - Unlocked (after commit_lock_file(), rollback_lock_file(), or a
* failed attempt to lock). In this state:
* - active is unset
- * - filename[0] == '\0' (usually, though there are transitory states
- * in which this condition doesn't hold)
+ * - filename is the empty string (usually, though there are
+ * transitory states in which this condition doesn't hold)
* - fd is -1
* - the object is left registered in the lock_file_list, and
* on_list is set.
@@ -180,13 +180,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);
@@ -201,29 +194,27 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
lk->active = 0;
lk->owner = 0;
lk->on_list = 1;
- lk->filename[0] = 0;
+ strbuf_init(&lk->filename, PATH_MAX);
lk->next = lock_file_list;
lock_file_list = lk;
}
- 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;
@@ -312,7 +303,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,14 +320,14 @@ int commit_lock_file(struct lock_file *lk)
goto rollback;
/* 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)
goto rollback;
lk->active = 0;
- lk->filename[0] = 0;
+ strbuf_reset(&lk->filename);
return 0;
rollback:
@@ -361,7 +352,7 @@ void rollback_lock_file(struct lock_file *lk)
if (lk->fd >= 0)
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 3d43dda..158241d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2026,10 +2026,10 @@ static int commit_locked_index(struct lock_file *lk)
if (alternate_index_output) {
if (lk->fd >= 0 && 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 f076f2d..717fc34 100644
--- a/refs.c
+++ b/refs.c
@@ -2551,8 +2551,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)
@@ -2918,7 +2918,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 de07709..33f3c7f 100644
--- 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"));
--
2.1.0
next prev parent reply other threads:[~2014-09-06 7:52 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-06 7:50 [PATCH v4 00/32] Lockfile correctness and refactoring Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 01/32] unable_to_lock_die(): rename function from unable_to_lock_index_die() Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 02/32] api-lockfile: expand the documentation Michael Haggerty
2014-09-09 22:40 ` Junio C Hamano
2014-09-06 7:50 ` [PATCH v4 03/32] rollback_lock_file(): do not clear filename redundantly Michael Haggerty
2014-09-11 19:13 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 04/32] rollback_lock_file(): exit early if lock is not active Michael Haggerty
2014-09-11 19:13 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 05/32] rollback_lock_file(): set fd to -1 Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 06/32] lockfile: unlock file if lockfile permissions cannot be adjusted Michael Haggerty
2014-09-09 22:39 ` Junio C Hamano
2014-09-12 11:03 ` Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 07/32] hold_lock_file_for_append(): release lock on errors Michael Haggerty
2014-09-09 22:41 ` Junio C Hamano
2014-09-12 11:04 ` Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 08/32] lock_file(): always add lock_file object to lock_file_list Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 09/32] lockfile.c: document the various states of lock_file objects Michael Haggerty
2014-09-11 19:57 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 10/32] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN Michael Haggerty
2014-09-11 22:15 ` Ronnie Sahlberg
2014-09-12 16:44 ` Michael Haggerty
2014-09-11 22:42 ` Ronnie Sahlberg
2014-09-12 17:13 ` Michael Haggerty
2014-09-12 17:32 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 11/32] delete_ref_loose(): don't muck around in the lock_file's filename Michael Haggerty
2014-09-13 7:41 ` Johannes Sixt
2014-09-14 6:27 ` Michael Haggerty
2014-09-14 6:38 ` Michael Haggerty
2014-09-14 14:49 ` Johannes Sixt
2014-09-06 7:50 ` [PATCH v4 12/32] prepare_index(): declare return value to be (const char *) Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 13/32] write_packed_entry_fn(): convert cb_data into a (const int *) Michael Haggerty
2014-09-11 19:55 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 14/32] lock_file(): exit early if lockfile cannot be opened Michael Haggerty
2014-09-11 22:49 ` Ronnie Sahlberg
2014-09-06 7:50 ` [PATCH v4 15/32] remove_lock_file(): call rollback_lock_file() Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 16/32] commit_lock_file(): inline temporary variable Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 17/32] commit_lock_file(): die() if called for unlocked lockfile object Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 18/32] commit_lock_file(): if close fails, roll back Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 19/32] commit_lock_file(): rollback lock file on failure to rename Michael Haggerty
2014-09-10 7:55 ` Jeff King
2014-09-10 12:55 ` Duy Nguyen
2014-09-06 7:50 ` [PATCH v4 20/32] api-lockfile: document edge cases Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 21/32] dump_marks(): remove a redundant call to rollback_lock_file() Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 22/32] git_config_set_multivar_in_file(): avoid " Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 23/32] lockfile: avoid transitory invalid states Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 24/32] struct lock_file: declare some fields volatile Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 25/32] try_merge_strategy(): remove redundant lock_file allocation Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 26/32] try_merge_strategy(): use a statically-allocated lock_file object Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 27/32] commit_lock_file(): use a strbuf to manage temporary space Michael Haggerty
2014-09-06 7:50 ` Michael Haggerty [this message]
2014-09-06 7:50 ` [PATCH v4 29/32] resolve_symlink(): use a strbuf for internal scratch space Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 30/32] resolve_symlink(): take a strbuf parameter Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 31/32] trim_last_path_elm(): replace last_path_elm() Michael Haggerty
2014-09-06 7:50 ` [PATCH v4 32/32] Extract a function commit_lock_file_to() Michael Haggerty
2014-09-07 14:21 ` [PATCH v4 00/32] Lockfile correctness and refactoring Torsten Bögershausen
2014-09-12 12:50 ` Michael Haggerty
2014-09-08 22:35 ` Junio C Hamano
2014-09-10 8:13 ` Jeff King
2014-09-10 10:25 ` Duy Nguyen
2014-09-10 10:30 ` Jeff King
2014-09-10 16:51 ` Junio C Hamano
2014-09-10 19:11 ` Jeff King
2014-09-12 11:28 ` Michael Haggerty
2014-09-12 11:13 ` Michael Haggerty
2014-09-12 14:21 ` Michael Haggerty
2014-09-13 18:51 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1409989846-22401-29-git-send-email-mhagger@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=peff@peff.net \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).