Re: [PATCH v3 3/4] close the index lock when not writing the new index

From: Junio C Hamano <gitster@pobox.com>
Date: May 29, 2017, 06:46 UTC
Message-ID: <xmqqo9uc6tzq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170528165642.14699-4-joel@teichroeb.net>

Joel Teichroeb <joel@teichroeb.net> writes:

> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>
> ---

The title says what the patch does; it does not explain why it is a good change. Lockfiles will be closed automatically when we exit anyway, so one can argue that the current code is good.

If you are planning to add more code to these "missing" else clauses in future steps in this series, this change makes sort-of sense. If that is the case, please say that in your log message.

Show 17 quoted lines

>  builtin/add.c     | 3 ++-
>  builtin/mv.c      | 8 +++++---
>  builtin/rm.c      | 3 ++-
>  merge-recursive.c | 8 +++++---
>  4 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 9f53f020d0..6b04eb2c71 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -461,7 +461,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	if (active_cache_changed) {
>  		if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
>  			die(_("Unable to write new index file"));
> -	}
> +	} else
> +		rollback_lock_file(&lock_file);

I think Ævar already pointed out the style issue, i.e.

	if (a-c-c) {
		if (w-l-i())
			die(...);
	} else {
		rollback_lock_file(&lock_file);
	}

The same for all other hunks in this patch.

Thanks.