On Wed, Apr 27, 2016 at 8:08 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> <christian.couder@xxxxxxxxx> 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@xxxxxxxxxxxxx>
>> ---
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html