Re: [PATCH 81/83] apply: roll back index in case of error

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 25, 2016, 16:06 UTC
Message-ID: <alpine.DEB.2.20.1604251802480.2896@virtualbox>
In-Reply-To: <1461505189-16234-2-git-send-email-chriscool@tuxfamily.org>

Hi Chris,

On Sun, 24 Apr 2016, Christian Couder wrote:

Show 12 quoted lines

> @@ -4734,16 +4737,22 @@ int apply_all_patches(struct apply_state *state,
>  		read_stdin = 0;
>  		set_default_whitespace_mode(state);
>  		res = apply_patch(state, fd, arg, options);
> -		if (res < 0)
> +		if (res < 0) {
> +			if (state->lock_file)
> +				rollback_lock_file(state->lock_file);
>  			return -1;
> +		}
>  		errs |= res;
>  		close(fd);

In case of error, this leaves fd open, which in the end will prevent the "patch" file, and hence the "rebase-apply/" directory from being removed on Windows. This triggered a failure of t4014 here (and possibly more, but it took me quite a while to track this down, what with builtin/am.c's am_destroy() not bothering at all to check the return value of remove_dir_recursively(), resulting in the error to be caught only much, much later).

Could you please review all open()/close() and fopen()/fclose() calls in your patch series, to make sure that there are no mistakes? A passing test suite does not really make me confident here, as our code coverage is not quite 100%.

Thanks, Dscho