Christian Couder <christian.couder@xxxxxxxxx> writes:
> In parse_binary() there is:
>
> forward = parse_binary_hunk(&buffer, &size, &status, &used);
> if (!forward && !status)
> /* there has to be one hunk (forward hunk) */
> return error(_("unrecognized binary patch at line %d"), linenr-1);
>
> so parse_binary() can return -1, because that's what error() returns.
>
> Also parse_binary_hunk() sets "status" to -1 in case of error and
> parse_binary() does "if (status) return status;".
All of the above sounds sensible, and your follow-up patch expects
parse_chunk() to return -1 on failure--it is a bit sad that you make
parse_chunk() to directly call exit(2). In the spirit of eventually
libifying this codepath, shouldn't we be turning existing die() to
an error return, instead of introducing more calls to exit(2)?
> In this case parse_chunk() should just exit, rather than add -1 to the
> patchsize it computes.
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
> builtin/apply.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 42c610e..18dec0f 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1872,6 +1872,11 @@ static struct fragment *parse_binary_hunk(char **buf_p,
> return NULL;
> }
>
> +/*
> + * Returns:
> + * -1 in case of error,
> + * the length of the parsed binary patch otherwise
> + */
> static int parse_binary(char *buffer, unsigned long size, struct patch *patch)
> {
> /*
> @@ -2017,6 +2022,8 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch)
> linenr++;
> used = parse_binary(buffer + hd + llen,
> size - hd - llen, patch);
> + if (used < 0)
> + exit(1);
> if (used)
> patchsize = used + llen;
> else
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html