Re: [PATCH v2 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag

[Junio C Hamano <gitster@pobox.com>]

Martin Ågren <martin.agren@gmail.com> writes:

> diff --git a/read-cache.c b/read-cache.c
> index 65f4fe837..1c917eba9 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2343,14 +2343,13 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
>  	int ret = do_write_index(istate, lock->tempfile, 0);
>  	if (ret)
>  		return ret;
> -	assert((flags & (COMMIT_LOCK | CLOSE_LOCK)) !=
> -	       (COMMIT_LOCK | CLOSE_LOCK));
>  	if (flags & COMMIT_LOCK)
>  		return commit_locked_index(lock);
> -	else if (flags & CLOSE_LOCK)
> -		return close_lock_file_gently(lock);
> -	else
> -		return ret;
> +	/*
> +	 * The lockfile already happens to have
> +	 * been closed, but let's be specific.
> +	 */
> +	return close_lock_file_gently(lock);

"already happens to have been" is quite a mouthful, and is not quite
truthful, as we do not foresee ever wanting to change that (because
of that stat(2) issue you mentioned).  It might be better to declare
that do_write_index() closes the lockfile after successfully writing
the data out to it.  I dunno if that reasoning is strong enough to
remove this (extra) close, though.

When any of the ce_write() calls in do_write_index() fails, the
function returns -1 without hitting the close/stat (obviously).
Somebody very high in the callchain (e.g. write_locked_index())
would clean it up by calling rollback_lock_file() eventually, so
that would not be a problem ;-)