Joel Teichroeb <joel@xxxxxxxxxxxxx> writes:
> Signed-off-by: Joel Teichroeb <joel@xxxxxxxxxxxxx>
> ---
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.
> 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.