@matheusbsbs / matheus-bagliano / commits / 6d058c8826

apply: move lockfile into `apply_state`

We have two users of `struct apply_state` and the related functionality in apply.c. Each user sets up its `apply_state` by handing over a pointer to its static `lock_file`. (Before 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we could never free lockfiles, so making them static was a reasonable approach.) Other than that, they never directly access their `lock_file`s, which are instead handled by the functionality in apply.c. To make life easier for the caller and to make it less tempting for a future caller to mess with the lock, make apply.c fully responsible for setting up the `lock_file`. As mentioned above, it is now safe to free a `lock_file`, so we can make the `struct apply_state` contain an actual `struct lock_file` instead of a pointer to one. The user in builtin/apply.c is rather simple. For builtin/am.c, we might worry that the lock state is actually meant to be inherited across calls. But the lock is only taken as `apply_all_patches()` executes, and code inspection shows that it will always be released. Alternatively, we can observe that the lock itself is never queried directly. When we decide whether we should lock, we check a related variable `newfd`. That variable is not inherited, so from the point of view of apply.c, the state machine really is reset with each call to `init_apply_state()`. (It would be a bug if `newfd` and the lock status were not in sync. The duplication of information in `newfd` and the lock will be addressed in the next patch.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Oct 5, 2017 at 22:32 UTC 6d058c882643dc45c8d708be98e86c38f25511a9

4 files changed +9 -17

apply.c

+5 -9

@@ -75,12 +75,10 @@ static int parse_ignorewhitespace_option(struct apply_state *state,

75 }

77 int init_apply_state(struct apply_state *state,

78 - const char *prefix,

79 - struct lock_file *lock_file)

78 + const char *prefix)

79 {

80 memset(state, 0, sizeof(*state));

81 state->prefix = prefix;

83 - state->lock_file = lock_file;

82 state->newfd = -1;

83 state->apply = 1;

84 state->line_termination = '\n';

@@ -146,8 +144,6 @@ int check_apply_state(struct apply_state *state, int force_apply)

144 }

145 if (state->check_index)

146 state->unsafe_paths = 0;

149 - if (!state->lock_file)

150 - return error("BUG: state->lock_file should not be NULL");

147

148 if (state->apply_verbosity <= verbosity_silent) {

149 state->saved_error_routine = get_error_routine();

@@ -4711,11 +4707,11 @@ static int apply_patch(struct apply_state *state,

4707 state->update_index = state->check_index && state->apply;

4708 if (state->update_index && state->newfd < 0) {

4709 if (state->index_file)

4714 - state->newfd = hold_lock_file_for_update(state->lock_file,

4710 + state->newfd = hold_lock_file_for_update(&state->lock_file,

4711 state->index_file,

4712 LOCK_DIE_ON_ERROR);

4713 else

4718 - state->newfd = hold_locked_index(state->lock_file, LOCK_DIE_ON_ERROR);

4714 + state->newfd = hold_locked_index(&state->lock_file, LOCK_DIE_ON_ERROR);

4715 }

4716

4717 if (state->check_index && read_apply_cache(state) < 0) {

@@ -4911,7 +4907,7 @@ int apply_all_patches(struct apply_state *state,

4907 }

4908

4909 if (state->update_index) {

4914 - res = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);

4910 + res = write_locked_index(&the_index, &state->lock_file, COMMIT_LOCK);

4911 if (res) {

4912 error(_("Unable to write new index file"));

4913 res = -128;

@@ -4924,7 +4920,7 @@ int apply_all_patches(struct apply_state *state,

4920

4921 end:

4922 if (state->newfd >= 0) {

4927 - rollback_lock_file(state->lock_file);

4923 + rollback_lock_file(&state->lock_file);

4924 state->newfd = -1;

4925 }

4926

apply.h

+2 -3

@@ -37,7 +37,7 @@ struct apply_state {

37 const char *prefix;

39 /* These are lock_file related */

40 - struct lock_file *lock_file;

40 + struct lock_file lock_file;

41 int newfd;

43 /* These control what gets looked at and modified */

@@ -116,8 +116,7 @@ extern int apply_parse_options(int argc, const char **argv,

116 int *force_apply, int *options,

117 const char * const *apply_usage);

118 extern int init_apply_state(struct apply_state *state,

119 - const char *prefix,

120 - struct lock_file *lock_file);

119 + const char *prefix);

120 extern void clear_apply_state(struct apply_state *state);

121 extern int check_apply_state(struct apply_state *state, int force_apply);

122

builtin/am.c

+1 -2

@@ -1488,11 +1488,10 @@ static int run_apply(const struct am_state *state, const char *index_file)

1488 struct argv_array apply_opts = ARGV_ARRAY_INIT;

1489 struct apply_state apply_state;

1490 int res, opts_left;

1491 - static struct lock_file lock_file;

1491 int force_apply = 0;

1492 int options = 0;

1493

1495 - if (init_apply_state(&apply_state, NULL, &lock_file))

1494 + if (init_apply_state(&apply_state, NULL))

1495 die("BUG: init_apply_state() failed");

1496

1497 argv_array_push(&apply_opts, "apply");

builtin/apply.c

+1 -3

@@ -9,8 +9,6 @@ static const char * const apply_usage[] = {

9 NULL

10 };

12 -static struct lock_file lock_file;

13 -

12 int cmd_apply(int argc, const char **argv, const char *prefix)

13 {

14 int force_apply = 0;

@@ -18,7 +16,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)

16 int ret;

17 struct apply_state state;

21 - if (init_apply_state(&state, prefix, &lock_file))

19 + if (init_apply_state(&state, prefix))

20 exit(128);

22 argc = apply_parse_options(argc, argv,