Skip to content

Commit 0b646bc

Browse files
gitstergitster
committed
Merge branch 'ma/lockfile-fixes'
An earlier update made it possible to use an on-stack in-core lockfile structure (as opposed to having to deliberately leak an on-heap one). Many codepaths have been updated to take advantage of this new facility. * ma/lockfile-fixes: read_cache: roll back lock in `update_index_if_able()` read-cache: leave lock in right state in `write_locked_index()` read-cache: drop explicit `CLOSE_LOCK`-flag cache.h: document `write_locked_index()` apply: remove `newfd` from `struct apply_state` apply: move lockfile into `apply_state` cache-tree: simplify locking logic checkout-index: simplify locking logic tempfile: fix documentation on `delete_tempfile()` lockfile: fix documentation on `close_lock_file_gently()` treewide: prefer lockfiles on the stack sha1_file: do not leak `lock_file`
2 parents cb5918a + b74c90f commit 0b646bc

21 files changed

+123
-131
lines changed

‎apply.c‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,10 @@ static int parse_ignorewhitespace_option(struct apply_state *state,
7575
}
7676

7777
int init_apply_state(struct apply_state *state,
78-
const char *prefix,
79-
struct lock_file *lock_file)
78+
const char *prefix)
8079
{
8180
memset(state, 0, sizeof(*state));
8281
state->prefix = prefix;
83-
state->lock_file = lock_file;
84-
state->newfd = -1;
8582
state->apply = 1;
8683
state->line_termination = '\n';
8784
state->p_value = 1;
@@ -146,8 +143,6 @@ int check_apply_state(struct apply_state *state, int force_apply)
146143
}
147144
if (state->check_index)
148145
state->unsafe_paths = 0;
149-
if (!state->lock_file)
150-
return error("BUG: state->lock_file should not be NULL");
151146

152147
if (state->apply_verbosity <= verbosity_silent) {
153148
state->saved_error_routine = get_error_routine();
@@ -4709,13 +4704,13 @@ static int apply_patch(struct apply_state *state,
47094704
state->apply = 0;
47104705

47114706
state->update_index = state->check_index && state->apply;
4712-
if (state->update_index && state->newfd < 0) {
4707+
if (state->update_index && !is_lock_file_locked(&state->lock_file)) {
47134708
if (state->index_file)
4714-
state->newfd = hold_lock_file_for_update(state->lock_file,
4715-
state->index_file,
4716-
LOCK_DIE_ON_ERROR);
4709+
hold_lock_file_for_update(&state->lock_file,
4710+
state->index_file,
4711+
LOCK_DIE_ON_ERROR);
47174712
else
4718-
state->newfd = hold_locked_index(state->lock_file, LOCK_DIE_ON_ERROR);
4713+
hold_locked_index(&state->lock_file, LOCK_DIE_ON_ERROR);
47194714
}
47204715

47214716
if (state->check_index && read_apply_cache(state) < 0) {
@@ -4911,22 +4906,18 @@ int apply_all_patches(struct apply_state *state,
49114906
}
49124907

49134908
if (state->update_index) {
4914-
res = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);
4909+
res = write_locked_index(&the_index, &state->lock_file, COMMIT_LOCK);
49154910
if (res) {
49164911
error(_("Unable to write new index file"));
49174912
res = -128;
49184913
goto end;
49194914
}
4920-
state->newfd = -1;
49214915
}
49224916

49234917
res = !!errs;
49244918

49254919
end:
4926-
if (state->newfd >= 0) {
4927-
rollback_lock_file(state->lock_file);
4928-
state->newfd = -1;
4929-
}
4920+
rollback_lock_file(&state->lock_file);
49304921

49314922
if (state->apply_verbosity <= verbosity_silent) {
49324923
set_error_routine(state->saved_error_routine);

‎apply.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ enum apply_verbosity {
3636
struct apply_state {
3737
const char *prefix;
3838

39-
/* These are lock_file related */
40-
struct lock_file *lock_file;
41-
int newfd;
39+
/* Lock file */
40+
struct lock_file lock_file;
4241

4342
/* These control what gets looked at and modified */
4443
int apply; /* this is not a dry-run */
@@ -116,8 +115,7 @@ extern int apply_parse_options(int argc, const char **argv,
116115
int *force_apply, int *options,
117116
const char * const *apply_usage);
118117
extern int init_apply_state(struct apply_state *state,
119-
const char *prefix,
120-
struct lock_file *lock_file);
118+
const char *prefix);
121119
extern void clear_apply_state(struct apply_state *state);
122120
extern int check_apply_state(struct apply_state *state, int force_apply);
123121

‎builtin/am.c‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,11 +1134,11 @@ static const char *msgnum(const struct am_state *state)
11341134
*/
11351135
static void refresh_and_write_cache(void)
11361136
{
1137-
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
1137+
struct lock_file lock_file = LOCK_INIT;
11381138

1139-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
1139+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
11401140
refresh_cache(REFRESH_QUIET);
1141-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
1141+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
11421142
die(_("unable to write index file"));
11431143
}
11441144

@@ -1488,11 +1488,10 @@ static int run_apply(const struct am_state *state, const char *index_file)
14881488
struct argv_array apply_opts = ARGV_ARRAY_INIT;
14891489
struct apply_state apply_state;
14901490
int res, opts_left;
1491-
static struct lock_file lock_file;
14921491
int force_apply = 0;
14931492
int options = 0;
14941493

1495-
if (init_apply_state(&apply_state, NULL, &lock_file))
1494+
if (init_apply_state(&apply_state, NULL))
14961495
die("BUG: init_apply_state() failed");
14971496

14981497
argv_array_push(&apply_opts, "apply");
@@ -1946,15 +1945,14 @@ static void am_resolve(struct am_state *state)
19461945
*/
19471946
static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
19481947
{
1949-
struct lock_file *lock_file;
1948+
struct lock_file lock_file = LOCK_INIT;
19501949
struct unpack_trees_options opts;
19511950
struct tree_desc t[2];
19521951

19531952
if (parse_tree(head) || parse_tree(remote))
19541953
return -1;
19551954

1956-
lock_file = xcalloc(1, sizeof(struct lock_file));
1957-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
1955+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
19581956

19591957
refresh_cache(REFRESH_QUIET);
19601958

@@ -1970,11 +1968,11 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
19701968
init_tree_desc(&t[1], remote->buffer, remote->size);
19711969

19721970
if (unpack_trees(2, t, &opts)) {
1973-
rollback_lock_file(lock_file);
1971+
rollback_lock_file(&lock_file);
19741972
return -1;
19751973
}
19761974

1977-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
1975+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
19781976
die(_("unable to write new index file"));
19791977

19801978
return 0;
@@ -1986,15 +1984,14 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
19861984
*/
19871985
static int merge_tree(struct tree *tree)
19881986
{
1989-
struct lock_file *lock_file;
1987+
struct lock_file lock_file = LOCK_INIT;
19901988
struct unpack_trees_options opts;
19911989
struct tree_desc t[1];
19921990

19931991
if (parse_tree(tree))
19941992
return -1;
19951993

1996-
lock_file = xcalloc(1, sizeof(struct lock_file));
1997-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
1994+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
19981995

19991996
memset(&opts, 0, sizeof(opts));
20001997
opts.head_idx = 1;
@@ -2005,11 +2002,11 @@ static int merge_tree(struct tree *tree)
20052002
init_tree_desc(&t[0], tree->buffer, tree->size);
20062003

20072004
if (unpack_trees(1, t, &opts)) {
2008-
rollback_lock_file(lock_file);
2005+
rollback_lock_file(&lock_file);
20092006
return -1;
20102007
}
20112008

2012-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
2009+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
20132010
die(_("unable to write new index file"));
20142011

20152012
return 0;

‎builtin/apply.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,14 @@ static const char * const apply_usage[] = {
99
NULL
1010
};
1111

12-
static struct lock_file lock_file;
13-
1412
int cmd_apply(int argc, const char **argv, const char *prefix)
1513
{
1614
int force_apply = 0;
1715
int options = 0;
1816
int ret;
1917
struct apply_state state;
2018

21-
if (init_apply_state(&state, prefix, &lock_file))
19+
if (init_apply_state(&state, prefix))
2220
exit(128);
2321

2422
argc = apply_parse_options(argc, argv,

‎builtin/checkout-index.c‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ static const char * const builtin_checkout_index_usage[] = {
129129
NULL
130130
};
131131

132-
static struct lock_file lock_file;
133-
134132
static int option_parse_stage(const struct option *opt,
135133
const char *arg, int unset)
136134
{
@@ -150,7 +148,7 @@ static int option_parse_stage(const struct option *opt,
150148
int cmd_checkout_index(int argc, const char **argv, const char *prefix)
151149
{
152150
int i;
153-
int newfd = -1;
151+
struct lock_file lock_file = LOCK_INIT;
154152
int all = 0;
155153
int read_from_stdin = 0;
156154
int prefix_length;
@@ -206,7 +204,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
206204
if (index_opt && !state.base_dir_len && !to_tempfile) {
207205
state.refresh_cache = 1;
208206
state.istate = &the_index;
209-
newfd = hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
207+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
210208
}
211209

212210
/* Check out named files first */
@@ -251,7 +249,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
251249
if (all)
252250
checkout_all(prefix, prefix_length);
253251

254-
if (0 <= newfd &&
252+
if (is_lock_file_locked(&lock_file) &&
255253
write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
256254
die("Unable to write new index file");
257255
return 0;

‎builtin/checkout.c‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ static int checkout_paths(const struct checkout_opts *opts,
247247
struct object_id rev;
248248
struct commit *head;
249249
int errs = 0;
250-
struct lock_file *lock_file;
250+
struct lock_file lock_file = LOCK_INIT;
251251

252252
if (opts->track != BRANCH_TRACK_UNSPECIFIED)
253253
die(_("'%s' cannot be used with updating paths"), "--track");
@@ -275,9 +275,7 @@ static int checkout_paths(const struct checkout_opts *opts,
275275
return run_add_interactive(revision, "--patch=checkout",
276276
&opts->pathspec);
277277

278-
lock_file = xcalloc(1, sizeof(struct lock_file));
279-
280-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
278+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
281279
if (read_cache_preload(&opts->pathspec) < 0)
282280
return error(_("index file corrupt"));
283281

@@ -376,7 +374,7 @@ static int checkout_paths(const struct checkout_opts *opts,
376374
}
377375
errs |= finish_delayed_checkout(&state);
378376

379-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
377+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
380378
die(_("unable to write new index file"));
381379

382380
read_ref_full("HEAD", 0, rev.hash, NULL);
@@ -472,9 +470,9 @@ static int merge_working_tree(const struct checkout_opts *opts,
472470
int *writeout_error)
473471
{
474472
int ret;
475-
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
473+
struct lock_file lock_file = LOCK_INIT;
476474

477-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
475+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
478476
if (read_cache_preload(NULL) < 0)
479477
return error(_("index file corrupt"));
480478

@@ -591,7 +589,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
591589
if (!cache_tree_fully_valid(active_cache_tree))
592590
cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
593591

594-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
592+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
595593
die(_("unable to write new index file"));
596594

597595
if (!opts->force && !opts->quiet)

‎builtin/clone.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ static int checkout(int submodule_progress)
706706
{
707707
struct object_id oid;
708708
char *head;
709-
struct lock_file *lock_file;
709+
struct lock_file lock_file = LOCK_INIT;
710710
struct unpack_trees_options opts;
711711
struct tree *tree;
712712
struct tree_desc t;
@@ -733,8 +733,7 @@ static int checkout(int submodule_progress)
733733
/* We need to be in the new work tree for the checkout */
734734
setup_work_tree();
735735

736-
lock_file = xcalloc(1, sizeof(struct lock_file));
737-
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
736+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
738737

739738
memset(&opts, 0, sizeof opts);
740739
opts.update = 1;
@@ -750,7 +749,7 @@ static int checkout(int submodule_progress)
750749
if (unpack_trees(1, &t, &opts) < 0)
751750
die(_("unable to checkout working tree"));
752751

753-
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
752+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
754753
die(_("unable to write new index file"));
755754

756755
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),

‎builtin/commit.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
355355

356356
refresh_cache_or_die(refresh_flags);
357357

358-
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
358+
if (write_locked_index(&the_index, &index_lock, 0))
359359
die(_("unable to create temporary index"));
360360

361361
old_index_env = getenv(INDEX_ENVIRONMENT);
@@ -374,7 +374,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
374374
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
375375
if (reopen_lock_file(&index_lock) < 0)
376376
die(_("unable to write index file"));
377-
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
377+
if (write_locked_index(&the_index, &index_lock, 0))
378378
die(_("unable to update temporary index"));
379379
} else
380380
warning(_("Failed to update main cache tree"));
@@ -401,7 +401,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
401401
add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
402402
refresh_cache_or_die(refresh_flags);
403403
update_main_cache_tree(WRITE_TREE_SILENT);
404-
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
404+
if (write_locked_index(&the_index, &index_lock, 0))
405405
die(_("unable to write new_index file"));
406406
commit_style = COMMIT_NORMAL;
407407
ret = get_lock_file_path(&index_lock);
@@ -474,7 +474,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
474474
add_remove_files(&partial);
475475
refresh_cache(REFRESH_QUIET);
476476
update_main_cache_tree(WRITE_TREE_SILENT);
477-
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
477+
if (write_locked_index(&the_index, &index_lock, 0))
478478
die(_("unable to write new_index file"));
479479

480480
hold_lock_file_for_update(&false_lock,
@@ -486,7 +486,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
486486
add_remove_files(&partial);
487487
refresh_cache(REFRESH_QUIET);
488488

489-
if (write_locked_index(&the_index, &false_lock, CLOSE_LOCK))
489+
if (write_locked_index(&the_index, &false_lock, 0))
490490
die(_("unable to write temporary index file"));
491491

492492
discard_cache();

‎builtin/diff.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,17 +203,16 @@ static int builtin_diff_combined(struct rev_info *revs,
203203

204204
static void refresh_index_quietly(void)
205205
{
206-
struct lock_file *lock_file;
206+
struct lock_file lock_file = LOCK_INIT;
207207
int fd;
208208

209-
lock_file = xcalloc(1, sizeof(struct lock_file));
210-
fd = hold_locked_index(lock_file, 0);
209+
fd = hold_locked_index(&lock_file, 0);
211210
if (fd < 0)
212211
return;
213212
discard_cache();
214213
read_cache();
215214
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
216-
update_index_if_able(&the_index, lock_file);
215+
update_index_if_able(&the_index, &lock_file);
217216
}
218217

219218
static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)

‎builtin/difftool.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
616616
if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||
617617
write_locked_index(&wtindex, &lock, COMMIT_LOCK)) {
618618
ret = error("could not write %s", buf.buf);
619-
rollback_lock_file(&lock);
620619
goto finish;
621620
}
622621
changed_files(&wt_modified, buf.buf, workdir);

0 commit comments

Comments
 (0)