On Wed, Apr 27, 2016 at 8:08 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> <christian.cou...@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 <chrisc...@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?

I think it is ok because find_header() is called only from
parse_chunk() which is only called by apply_patch() and apply_patch()
calls "free_patch(patch)" when parse_chunk() returns a negative
integer.
And free_patch() frees old_name and new_name.
--
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

Reply via email to