Martin Ågren <martin.agren@xxxxxxxxx> writes:
> 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);
> }
This is not a show-stopper for this patch series, but just something
I noticed, something that used to be unavoidable in the old world
order that requires us to leak lockfiles, but now it becomes more
obvious.
update_index_if_able() calls write_lock_index() with the COMMIT_LOCK
option, but it does so conditionally. When it does not make the call,
the lockfile is left behind to be cleaned up by the atexit() handler,
but when it does, it is closed and released.
Perhaps update_index_if_able() needs to be further cleaned up to
always release the resource held by the lockfile structure? Its
caller cannot differentiate (and more importantly, its caller does
not want to care) if the _if_able call actually has updated the
index or not and act differently afterwards.
