Re: [PATCH 2/2] apply: handle assertion failure gracefully

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

René Scharfe <l.s.r@web.de> writes:

> Thought a bit more about it, and as a result here's a simpler approach:
>
> -- >8 --
> Subject: [PATCH] apply: check git diffs for mutually exclusive header lines
>
> A file can either be added, removed, copied, or renamed, but no two of
> these actions can be done by the same patch.  Some of these combinations
> provoke error messages due to missing file names, and some are only
> caught by an assertion.  Check git patches already as they are parsed
> and report conflicting lines on sight.
>
> Found by Vegard Nossum using AFL.
>
> Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  apply.c                | 14 ++++++++++++++
>  apply.h                |  1 +
>  t/t4136-apply-check.sh | 18 ++++++++++++++++++
>  3 files changed, 33 insertions(+)
>
> diff --git a/apply.c b/apply.c
> index 8cd6435c74..8a5e44c474 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -1312,6 +1312,18 @@ static char *git_header_name(struct apply_state *state,
>  	}
>  }
>  
> +static int check_header_line(struct apply_state *state, struct patch *patch)
> +{
> +	int extensions = (patch->is_delete == 1) + (patch->is_new == 1) +
> +			 (patch->is_rename == 1) + (patch->is_copy == 1);
> +	if (extensions > 1)
> +		return error(_("inconsistent header lines %d and %d"),
> +			     state->extension_linenr, state->linenr);
> +	if (extensions && !state->extension_linenr)
> +		state->extension_linenr = state->linenr;

OK.  I wondered briefly what happens if the first git_header that
sets one of the extensions can be at line 0 (calusng
state->extension_linenr to be set to 0), but even in that case, the
second problematic one will correctly report the 0th and its own
line as culprit, so this is OK.  It makes me question if there is
any point checking !state->extension_linenr in the if() statement,
though.