From: Eric Sunshine Date: Mon, 02 May 2016 07:32:06 GMT Subject: Re: [PATCH 67/83] builtin/apply: make build_fake_ancestor() return -1 on error Message-ID: In-Reply-To: <1461504863-15946-68-git-send-email-chriscool@tuxfamily.org> On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/builtin/apply.c b/builtin/apply.c > @@ -3913,31 +3913,34 @@ static void build_fake_ancestor(struct patch *list, const char *filename) > ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0); > if (!ce) > - die(_("make_cache_entry failed for path '%s'"), name); > + return error(_("make_cache_entry failed for path '%s'"), > + name); > if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD)) > - die ("Could not add %s to temporary index", name); > + return error("Could not add %s to temporary index", > + name); What happens to 'ce' here? Does it leak? > } > > hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR); > if (write_locked_index(&result, &lock, COMMIT_LOCK)) > - die ("Could not write temporary index to %s", filename); > + return error("Could not write temporary index to %s", filename); What happens with 'result'? Does it need to be discarded? > discard_index(&result); > + return 0; > } > > static void stat_patch_list(struct apply_state *state, struct patch *patch) > @@ -4470,8 +4473,9 @@ static int apply_patch(struct apply_state *state, > return 1; > } > > - if (state->fake_ancestor) > - build_fake_ancestor(list, state->fake_ancestor); > + if (state->fake_ancestor && > + build_fake_ancestor(list, state->fake_ancestor)) > + return -1; Same comment as earlier patches: Leaking 'list', 'buf', 'fn_table'. > if (state->diffstat) > stat_patch_list(state, list);