Re: [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error

[Christian Couder]

On Mon, May 16, 2016 at 5:04 AM, Eric Sunshine  wrote:
> On Wed, May 11, 2016 at 9:17 AM, Christian Couder
>  wrote:
>> To libify `git apply` functionality we have to signal errors to the
>> caller instead of die()ing or exit()ing.
>>
>> To do that in a compatible manner with the rest of the error handling
>> in builtin/apply.c, find_header() should return -1 instead of calling
>> die() or exit().
>
> Why is this talking about making find_header() return -1? Didn't that
> happen in the previous patch?

Yeah, it should be parse_chunk() not find_header().

This is fixed in my current branch.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error

[Christian Couder]

On Mon, May 16, 2016 at 5:04 AM, Eric Sunshine  wrote:
> On Wed, May 11, 2016 at 9:17 AM, Christian Couder
>  wrote:
>> To libify `git apply` functionality we have to signal errors to the
>> caller instead of die()ing or exit()ing.
>>
>> To do that in a compatible manner with the rest of the error handling
>> in builtin/apply.c, find_header() should return -1 instead of calling
>> die() or exit().
>
> Why is this talking about making find_header() return -1? Didn't that
> happen in the previous patch?
>
>> As parse_chunk() is called only by apply_patch() which already
>> returns -1 when an error happened, let's make apply_patch() return -1
>> when parse_chunk() returns -1.
>>
>> If find_header() returns -2 because no patch header has been found, it
>> is ok for parse_chunk() to also return -2. If find_header() returns -1
>> because an error happened, it is ok for parse_chunk() to do the same.
>>
>> Helped-by: Eric Sunshine 
>> Signed-off-by: Christian Couder 
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -2176,8 +2176,9 @@ static int parse_chunk(struct apply_state *state, char 
>> *buffer, unsigned long si
>>  * empty to us here.
>>  */
>> if ((state->apply || state->check) &&
>> -   (!patch->is_binary && !metadata_changes(patch)))
>> -   die(_("patch with only garbage at line %d"), 
>> state->linenr);
>> +   (!patch->is_binary && !metadata_changes(patch))) {
>> +   return error(_("patch with only garbage at line 
>> %d"), state->linenr);
>> +   }
>
> Unnecessary braces.

Ok, will remove.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error

[Eric Sunshine]

On Wed, May 11, 2016 at 9:17 AM, Christian Couder
 wrote:
> To libify `git apply` functionality we have to signal errors to the
> caller instead of die()ing or exit()ing.
>
> To do that in a compatible manner with the rest of the error handling
> in builtin/apply.c, find_header() should return -1 instead of calling
> die() or exit().

Why is this talking about making find_header() return -1? Didn't that
happen in the previous patch?

> As parse_chunk() is called only by apply_patch() which already
> returns -1 when an error happened, let's make apply_patch() return -1
> when parse_chunk() returns -1.
>
> If find_header() returns -2 because no patch header has been found, it
> is ok for parse_chunk() to also return -2. If find_header() returns -1
> because an error happened, it is ok for parse_chunk() to do the same.
>
> Helped-by: Eric Sunshine 
> Signed-off-by: Christian Couder 
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -2176,8 +2176,9 @@ static int parse_chunk(struct apply_state *state, char 
> *buffer, unsigned long si
>  * empty to us here.
>  */
> if ((state->apply || state->check) &&
> -   (!patch->is_binary && !metadata_changes(patch)))
> -   die(_("patch with only garbage at line %d"), 
> state->linenr);
> +   (!patch->is_binary && !metadata_changes(patch))) {
> +   return error(_("patch with only garbage at line %d"), 
> state->linenr);
> +   }

Unnecessary braces.

> }
>
> return offset + hdrsize + patchsize;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html