Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> To be compatible with the rest of the error handling in builtin/apply.c,
>> find_header() should return -1 instead of calling die().
>>
>> Unfortunately find_header() already returns -1 when no header is found,
>> so let's make it return -2 instead in this case.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -1588,18 +1596,18 @@ static int find_header(struct apply_state *state,
>>                                 continue;
>>                         if (!patch->old_name && !patch->new_name) {
>>                                 if (!patch->def_name)
>> -                                       die(Q_("git diff header lacks filename information when removing "
>> -                                              "%d leading pathname component (line %d)",
>> -                                              "git diff header lacks filename information when removing "
>> -                                              "%d leading pathname components (line %d)",
>> -                                              state->p_value),
>> -                                           state->p_value, state->linenr);
>> +                                       return error(Q_("git diff header lacks filename information when removing "
>> +                                                       "%d leading pathname component (line %d)",
>> +                                                       "git diff header lacks filename information when removing "
>> +                                                       "%d leading pathname components (line %d)",
>> +                                                       state->p_value),
>> +                                                    state->p_value, state->linenr);
>>                                 patch->old_name = xstrdup(patch->def_name);
>>                                 patch->new_name = xstrdup(patch->def_name);
>>                         }
>>                         if (!patch->is_delete && !patch->new_name)
>> -                               die("git diff header lacks filename information "
>> -                                   "(line %d)", state->linenr);
>> +                               return error("git diff header lacks filename information "
>> +                                            "(line %d)", state->linenr);
>
> I realize that the caller in this patch currently just die()'s, and I
> haven't looked at subsequent patches yet, but is this new 'return'
> going to cause the caller to start leaking patch->old_name and
> patch->new_name which are xstrdup()'d just above?