From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 02/12] treewide: prefer lockfiles on the stack
Date: Thu, 5 Oct 2017 22:32:04 +0200 [thread overview]
Message-ID: <2fdb048f30131b30ea910018719485fc8cff4b11.1507228170.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1507228170.git.martin.agren@gmail.com>
There is no longer any need to allocate and leak a `struct lock_file`.
The previous patch addressed an instance where we needed a minor tweak
alongside the trivial changes.
Deal with the remaining instances where we allocate and leak a struct
within a single function. Change them to have the `struct lock_file` on
the stack instead.
These instances were identified by running `git grep "^\s*struct
lock_file\s*\*"`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
v2: Identical.
builtin/am.c | 24 +++++++++++-------------
builtin/checkout.c | 14 ++++++--------
builtin/clone.c | 7 +++----
builtin/diff.c | 7 +++----
config.c | 17 ++++++++---------
merge-recursive.c | 6 +++---
merge.c | 8 ++++----
wt-status.c | 8 ++++----
8 files changed, 42 insertions(+), 49 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d7513f537..4e16fd428 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1134,11 +1134,11 @@ static const char *msgnum(const struct am_state *state)
*/
static void refresh_and_write_cache(void)
{
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+ struct lock_file lock_file = LOCK_INIT;
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
refresh_cache(REFRESH_QUIET);
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write index file"));
}
@@ -1946,15 +1946,14 @@ static void am_resolve(struct am_state *state)
*/
static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
{
- struct lock_file *lock_file;
+ struct lock_file lock_file = LOCK_INIT;
struct unpack_trees_options opts;
struct tree_desc t[2];
if (parse_tree(head) || parse_tree(remote))
return -1;
- lock_file = xcalloc(1, sizeof(struct lock_file));
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
refresh_cache(REFRESH_QUIET);
@@ -1970,11 +1969,11 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
init_tree_desc(&t[1], remote->buffer, remote->size);
if (unpack_trees(2, t, &opts)) {
- rollback_lock_file(lock_file);
+ rollback_lock_file(&lock_file);
return -1;
}
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
return 0;
@@ -1986,15 +1985,14 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
*/
static int merge_tree(struct tree *tree)
{
- struct lock_file *lock_file;
+ struct lock_file lock_file = LOCK_INIT;
struct unpack_trees_options opts;
struct tree_desc t[1];
if (parse_tree(tree))
return -1;
- lock_file = xcalloc(1, sizeof(struct lock_file));
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
memset(&opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -2005,11 +2003,11 @@ static int merge_tree(struct tree *tree)
init_tree_desc(&t[0], tree->buffer, tree->size);
if (unpack_trees(1, t, &opts)) {
- rollback_lock_file(lock_file);
+ rollback_lock_file(&lock_file);
return -1;
}
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
return 0;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3345a0d16..34c3c5b61 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -247,7 +247,7 @@ static int checkout_paths(const struct checkout_opts *opts,
struct object_id rev;
struct commit *head;
int errs = 0;
- struct lock_file *lock_file;
+ struct lock_file lock_file = LOCK_INIT;
if (opts->track != BRANCH_TRACK_UNSPECIFIED)
die(_("'%s' cannot be used with updating paths"), "--track");
@@ -275,9 +275,7 @@ static int checkout_paths(const struct checkout_opts *opts,
return run_add_interactive(revision, "--patch=checkout",
&opts->pathspec);
- lock_file = xcalloc(1, sizeof(struct lock_file));
-
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
if (read_cache_preload(&opts->pathspec) < 0)
return error(_("index file corrupt"));
@@ -376,7 +374,7 @@ static int checkout_paths(const struct checkout_opts *opts,
}
errs |= finish_delayed_checkout(&state);
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
read_ref_full("HEAD", 0, rev.hash, NULL);
@@ -472,9 +470,9 @@ static int merge_working_tree(const struct checkout_opts *opts,
int *writeout_error)
{
int ret;
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+ struct lock_file lock_file = LOCK_INIT;
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
if (read_cache_preload(NULL) < 0)
return error(_("index file corrupt"));
@@ -591,7 +589,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
if (!cache_tree_fully_valid(active_cache_tree))
cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
if (!opts->force && !opts->quiet)
diff --git a/builtin/clone.c b/builtin/clone.c
index dbddd98f8..96a3aaaa1 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -706,7 +706,7 @@ static int checkout(int submodule_progress)
{
struct object_id oid;
char *head;
- struct lock_file *lock_file;
+ struct lock_file lock_file = LOCK_INIT;
struct unpack_trees_options opts;
struct tree *tree;
struct tree_desc t;
@@ -733,8 +733,7 @@ static int checkout(int submodule_progress)
/* We need to be in the new work tree for the checkout */
setup_work_tree();
- lock_file = xcalloc(1, sizeof(struct lock_file));
- hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
memset(&opts, 0, sizeof opts);
opts.update = 1;
@@ -750,7 +749,7 @@ static int checkout(int submodule_progress)
if (unpack_trees(1, &t, &opts) < 0)
die(_("unable to checkout working tree"));
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
diff --git a/builtin/diff.c b/builtin/diff.c
index 7e3ebcea3..91995965d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -203,17 +203,16 @@ static int builtin_diff_combined(struct rev_info *revs,
static void refresh_index_quietly(void)
{
- struct lock_file *lock_file;
+ struct lock_file lock_file = LOCK_INIT;
int fd;
- lock_file = xcalloc(1, sizeof(struct lock_file));
- fd = hold_locked_index(lock_file, 0);
+ fd = hold_locked_index(&lock_file, 0);
if (fd < 0)
return;
discard_cache();
read_cache();
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
- update_index_if_able(&the_index, lock_file);
+ update_index_if_able(&the_index, &lock_file);
}
static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)
diff --git a/config.c b/config.c
index 345d78c2b..0e61e863d 100644
--- a/config.c
+++ b/config.c
@@ -2748,7 +2748,7 @@ int git_config_rename_section_in_file(const char *config_filename,
{
int ret = 0, remove = 0;
char *filename_buf = NULL;
- struct lock_file *lock;
+ struct lock_file lock = LOCK_INIT;
int out_fd;
char buf[1024];
FILE *config_file = NULL;
@@ -2762,8 +2762,7 @@ int git_config_rename_section_in_file(const char *config_filename,
if (!config_filename)
config_filename = filename_buf = git_pathdup("config");
- lock = xcalloc(1, sizeof(struct lock_file));
- out_fd = hold_lock_file_for_update(lock, config_filename, 0);
+ out_fd = hold_lock_file_for_update(&lock, config_filename, 0);
if (out_fd < 0) {
ret = error("could not lock config file %s", config_filename);
goto out;
@@ -2782,9 +2781,9 @@ int git_config_rename_section_in_file(const char *config_filename,
goto out;
}
- if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
+ if (chmod(get_lock_file_path(&lock), st.st_mode & 07777) < 0) {
ret = error_errno("chmod on %s failed",
- get_lock_file_path(lock));
+ get_lock_file_path(&lock));
goto out;
}
@@ -2805,7 +2804,7 @@ int git_config_rename_section_in_file(const char *config_filename,
}
store.baselen = strlen(new_name);
if (write_section(out_fd, new_name) < 0) {
- ret = write_error(get_lock_file_path(lock));
+ ret = write_error(get_lock_file_path(&lock));
goto out;
}
/*
@@ -2831,20 +2830,20 @@ int git_config_rename_section_in_file(const char *config_filename,
continue;
length = strlen(output);
if (write_in_full(out_fd, output, length) < 0) {
- ret = write_error(get_lock_file_path(lock));
+ ret = write_error(get_lock_file_path(&lock));
goto out;
}
}
fclose(config_file);
config_file = NULL;
commit_and_out:
- if (commit_lock_file(lock) < 0)
+ if (commit_lock_file(&lock) < 0)
ret = error_errno("could not write config file %s",
config_filename);
out:
if (config_file)
fclose(config_file);
- rollback_lock_file(lock);
+ rollback_lock_file(&lock);
out_no_rollback:
free(filename_buf);
return ret;
diff --git a/merge-recursive.c b/merge-recursive.c
index 1d3f8f0d2..24c5c26a6 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2162,7 +2162,7 @@ int merge_recursive_generic(struct merge_options *o,
struct commit **result)
{
int clean;
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ struct lock_file lock = LOCK_INIT;
struct commit *head_commit = get_ref(head, o->branch1);
struct commit *next_commit = get_ref(merge, o->branch2);
struct commit_list *ca = NULL;
@@ -2178,14 +2178,14 @@ int merge_recursive_generic(struct merge_options *o,
}
}
- hold_locked_index(lock, LOCK_DIE_ON_ERROR);
+ hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
clean = merge_recursive(o, head_commit, next_commit, ca,
result);
if (clean < 0)
return clean;
if (active_cache_changed &&
- write_locked_index(&the_index, lock, COMMIT_LOCK))
+ write_locked_index(&the_index, &lock, COMMIT_LOCK))
return err(o, _("Unable to write index."));
return clean ? 0 : 1;
diff --git a/merge.c b/merge.c
index 1d441ad94..a18a452b5 100644
--- a/merge.c
+++ b/merge.c
@@ -53,11 +53,11 @@ int checkout_fast_forward(const struct object_id *head,
struct tree_desc t[MAX_UNPACK_TREES];
int i, nr_trees = 0;
struct dir_struct dir;
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+ struct lock_file lock_file = LOCK_INIT;
refresh_cache(REFRESH_QUIET);
- if (hold_locked_index(lock_file, LOCK_REPORT_ON_ERROR) < 0)
+ if (hold_locked_index(&lock_file, LOCK_REPORT_ON_ERROR) < 0)
return -1;
memset(&trees, 0, sizeof(trees));
@@ -91,8 +91,8 @@ int checkout_fast_forward(const struct object_id *head,
}
if (unpack_trees(nr_trees, t, &opts))
return -1;
- if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) {
- rollback_lock_file(lock_file);
+ if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) {
+ rollback_lock_file(&lock_file);
return error(_("unable to write new index file"));
}
return 0;
diff --git a/wt-status.c b/wt-status.c
index 6f730ee8f..7d88e7ca8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2299,14 +2299,14 @@ int has_uncommitted_changes(int ignore_submodules)
*/
int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently)
{
- struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+ struct lock_file lock_file = LOCK_INIT;
int err = 0, fd;
- fd = hold_locked_index(lock_file, 0);
+ fd = hold_locked_index(&lock_file, 0);
refresh_cache(REFRESH_QUIET);
if (0 <= fd)
- update_index_if_able(&the_index, lock_file);
- rollback_lock_file(lock_file);
+ update_index_if_able(&the_index, &lock_file);
+ rollback_lock_file(&lock_file);
if (has_unstaged_changes(ignore_submodules)) {
/* TRANSLATORS: the action is e.g. "pull with rebase" */
--
2.14.2.666.gea220ee40
next prev parent reply other threads:[~2017-10-05 20:32 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-01 14:56 [PATCH 00/11] various lockfile-leaks and -fixes Martin Ågren
2017-10-01 14:56 ` [PATCH 01/11] sha1_file: do not leak `lock_file` Martin Ågren
2017-10-02 5:26 ` Jeff King
2017-10-02 10:15 ` Martin Ågren
2017-10-01 14:56 ` [PATCH 02/11] treewide: prefer lockfiles on the stack Martin Ågren
2017-10-02 3:37 ` Junio C Hamano
2017-10-02 4:12 ` Martin Ågren
2017-10-02 5:34 ` Jeff King
2017-10-01 14:56 ` [PATCH 03/11] lockfile: fix documentation on `close_lock_file_gently()` Martin Ågren
2017-10-02 5:35 ` Jeff King
2017-10-01 14:56 ` [PATCH 04/11] tempfile: fix documentation on `delete_tempfile()` Martin Ågren
2017-10-02 5:38 ` Jeff King
2017-10-01 14:56 ` [PATCH 05/11] cache-tree: simplify locking logic Martin Ågren
2017-10-02 3:40 ` Junio C Hamano
2017-10-02 5:41 ` Jeff King
2017-10-01 14:56 ` [PATCH 06/11] apply: move lockfile into `apply_state` Martin Ågren
2017-10-02 5:48 ` Jeff King
2017-10-01 14:56 ` [PATCH 07/11] apply: remove `newfd` from `struct apply_state` Martin Ågren
2017-10-02 5:50 ` Jeff King
2017-10-01 14:56 ` [PATCH 08/11] cache.h: document `write_locked_index()` Martin Ågren
2017-10-01 14:56 ` [PATCH 09/11] read-cache: require flags for `write_locked_index()` Martin Ågren
2017-10-02 3:49 ` Junio C Hamano
2017-10-02 4:14 ` Martin Ågren
2017-10-02 10:16 ` Martin Ågren
2017-10-02 6:00 ` Jeff King
2017-10-01 14:56 ` [PATCH 10/11] read-cache: don't leave dangling pointer in `do_write_index()` Martin Ågren
2017-10-02 6:15 ` Jeff King
2017-10-02 6:20 ` Jeff King
2017-10-01 14:56 ` [PATCH 11/11] read-cache: roll back lock on error with `COMMIT_LOCK` Martin Ågren
2017-10-02 4:01 ` Junio C Hamano
2017-10-02 2:37 ` [PATCH 00/11] various lockfile-leaks and -fixes Junio C Hamano
2017-10-02 6:22 ` Jeff King
2017-10-02 6:30 ` Junio C Hamano
2017-10-02 10:19 ` Martin Ågren
2017-10-03 6:21 ` Junio C Hamano
2017-10-05 20:32 ` [PATCH v2 00/12] " Martin Ågren
2017-10-05 20:32 ` [PATCH v2 01/12] sha1_file: do not leak `lock_file` Martin Ågren
2017-10-06 1:17 ` Junio C Hamano
2017-10-05 20:32 ` Martin Ågren [this message]
2017-10-05 20:32 ` [PATCH v2 03/12] lockfile: fix documentation on `close_lock_file_gently()` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 04/12] tempfile: fix documentation on `delete_tempfile()` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 05/12] checkout-index: simplify locking logic Martin Ågren
2017-10-06 1:21 ` Junio C Hamano
2017-10-05 20:32 ` [PATCH v2 06/12] cache-tree: " Martin Ågren
2017-10-05 20:32 ` [PATCH v2 07/12] apply: move lockfile into `apply_state` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 08/12] apply: remove `newfd` from `struct apply_state` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 09/12] cache.h: document `write_locked_index()` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag Martin Ågren
2017-10-06 1:39 ` Junio C Hamano
2017-10-06 11:02 ` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 11/12] read-cache: leave lock in right state in `write_locked_index()` Martin Ågren
2017-10-06 2:01 ` Junio C Hamano
2017-10-06 11:04 ` Martin Ågren
2017-10-06 12:02 ` Junio C Hamano
2017-10-06 19:44 ` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 00/12] Re: various lockfile-leaks and -fixes Martin Ågren
2017-10-06 20:12 ` [PATCH v3 01/12] sha1_file: do not leak `lock_file` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 02/12] treewide: prefer lockfiles on the stack Martin Ågren
2017-10-06 20:12 ` [PATCH v3 03/12] lockfile: fix documentation on `close_lock_file_gently()` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 04/12] tempfile: fix documentation on `delete_tempfile()` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 05/12] checkout-index: simplify locking logic Martin Ågren
2017-10-06 20:12 ` [PATCH v3 06/12] cache-tree: " Martin Ågren
2017-10-06 20:12 ` [PATCH v3 07/12] apply: move lockfile into `apply_state` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 08/12] apply: remove `newfd` from `struct apply_state` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 09/12] cache.h: document `write_locked_index()` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag Martin Ågren
2017-10-06 20:12 ` [PATCH v3 11/12] read-cache: leave lock in right state in `write_locked_index()` Martin Ågren
2017-10-06 20:12 ` [PATCH v3 12/12] read_cache: roll back lock in `update_index_if_able()` Martin Ågren
2017-10-05 20:32 ` [PATCH v2 " Martin Ågren
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2fdb048f30131b30ea910018719485fc8cff4b11.1507228170.git.martin.agren@gmail.com \
--to=martin.agren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).