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?

>                         patch->is_toplevel_relative = 1;
>                         *hdrsize = git_hdr_len;
>                         return offset;
> @@ -2115,6 +2123,9 @@ static int parse_chunk(struct apply_state *state, char 
> *buffer, unsigned long si
>         int hdrsize, patchsize;
>         int offset = find_header(state, buffer, size, &hdrsize, patch);
>
> +       if (offset == -1)
> +               exit(1);
> +
>         if (offset < 0)
>                 return offset;
--
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