On Wed, Apr 27, 2016 at 8:08 PM, Eric Sunshine [off-list ref] wrote:

On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
[off-list ref] wrote:

quoted

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 <redacted>
---

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.