Re: [PATCH v6 5/6] convert: move multiple file filter error handling to separate function

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

Lars Schneider <larsxschneider@gmail.com> writes:

> Refactoring the filter error handling is useful for the subsequent patch
> 'convert: add "status=delayed" to filter process protocol'.
>
> In addition, replace the parentheses around the empty "if" block with a
> single semicolon to adhere to the Git style guide.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  convert.c | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)

The patch looks obviously correct.  Thanks.

> diff --git a/convert.c b/convert.c
> index 9907e3b9ba..e55c034d86 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -565,6 +565,29 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
>  	return err;
>  }
>  
> +static void handle_filter_error(const struct strbuf *filter_status,
> +				struct cmd2process *entry,
> +				const unsigned int wanted_capability) {
> +	if (!strcmp(filter_status->buf, "error"))
> +		; /* The filter signaled a problem with the file. */
> +	else if (!strcmp(filter_status->buf, "abort") && wanted_capability) {
> +		/*
> +		 * The filter signaled a permanent problem. Don't try to filter
> +		 * files with the same command for the lifetime of the current
> +		 * Git process.
> +		 */
> +		 entry->supported_capabilities &= ~wanted_capability;
> +	} else {
> +		/*
> +		 * Something went wrong with the protocol filter.
> +		 * Force shutdown and restart if another blob requires filtering.
> +		 */
> +		error("external filter '%s' failed", entry->subprocess.cmd);
> +		subprocess_stop(&subprocess_map, &entry->subprocess);
> +		free(entry);
> +	}
> +}
> +
>  static int apply_multi_file_filter(const char *path, const char *src, size_t len,
>  				   int fd, struct strbuf *dst, const char *cmd,
>  				   const unsigned int wanted_capability)
> @@ -656,28 +679,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
>  done:
>  	sigchain_pop(SIGPIPE);
>  
> -	if (err) {
> -		if (!strcmp(filter_status.buf, "error")) {
> -			/* The filter signaled a problem with the file. */
> -		} else if (!strcmp(filter_status.buf, "abort")) {
> -			/*
> -			 * The filter signaled a permanent problem. Don't try to filter
> -			 * files with the same command for the lifetime of the current
> -			 * Git process.
> -			 */
> -			 entry->supported_capabilities &= ~wanted_capability;
> -		} else {
> -			/*
> -			 * Something went wrong with the protocol filter.
> -			 * Force shutdown and restart if another blob requires filtering.
> -			 */
> -			error("external filter '%s' failed", cmd);
> -			subprocess_stop(&subprocess_map, &entry->subprocess);
> -			free(entry);
> -		}
> -	} else {
> +	if (err)
> +		handle_filter_error(&filter_status, entry, wanted_capability);
> +	else
>  		strbuf_swap(dst, &nbuf);
> -	}
>  	strbuf_release(&nbuf);
>  	return !err;
>  }