From: Elijah Newren <new...@gmail.com>

Hi Elijah,

[Since this is leaving the topic of rename-detection in favour of 
leak-plugging, I'm shortening the cc-list a bit.]

> So, instead, I'd like to see something like the below
> (built on top of my series):

Thanks a lot. I now have the below patch in my tree as a preparatory
part of a three-patch series on top of your series. Since the gist of
this patch is entirely your creation, is it ok if I place your Author:
and Signed-off-by: on it? Credit where credit is due.

As you noted elsewhere [1], Ben is also working in this area. I'd be
perfectly happy to sit on these patches until both of your contributions
come through to master.

[1] 
https://public-inbox.org/git/CABPp-BFh=gl6rnbst2bgtynkij1z5tmgar1via5_vytef5e...@mail.gmail.com/

Martin

-->8--
Subject: merge-recursive: provide pair of `unpack_trees_{start,finish}()`

Rename `git_merge_trees()` to `unpack_trees_start()` and extract the
call to `discard_index()` into a new function `unpack_trees_finish()`.
As a result, these are called early resp. late in `merge_trees()`,
making the resource handling clearer. The next commit will expand on
that, teaching `..._finish()` to free more memory. (So rather than
moving the TODO-comment, just drop it, since it will be addressed soon
enough.)

Also call `..._finish()` when `merge_trees()` returns early.
---
 merge-recursive.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 1de8dc1c53..e64102004a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -337,10 +337,10 @@ static void init_tree_desc_from_tree(struct tree_desc 
*desc, struct tree *tree)
        init_tree_desc(desc, tree->buffer, tree->size);
 }
 
-static int git_merge_trees(struct merge_options *o,
-                          struct tree *common,
-                          struct tree *head,
-                          struct tree *merge)
+static int unpack_trees_start(struct merge_options *o,
+                             struct tree *common,
+                             struct tree *head,
+                             struct tree *merge)
 {
        int rc;
        struct tree_desc t[3];
@@ -378,6 +378,11 @@ static int git_merge_trees(struct merge_options *o,
        return rc;
 }
 
+static void unpack_trees_finish(struct merge_options *o)
+{
+       discard_index(&o->orig_index);
+}
+
 struct tree *write_tree_from_memory(struct merge_options *o)
 {
        struct tree *result = NULL;
@@ -3079,13 +3084,14 @@ int merge_trees(struct merge_options *o,
                return 1;
        }
 
-       code = git_merge_trees(o, common, head, merge);
+       code = unpack_trees_start(o, common, head, merge);
 
        if (code != 0) {
                if (show(o, 4) || o->call_depth)
                        err(o, _("merging of trees %s and %s failed"),
                            oid_to_hex(&head->object.oid),
                            oid_to_hex(&merge->object.oid));
+               unpack_trees_finish(o);
                return -1;
        }
 
@@ -3138,20 +3144,15 @@ int merge_trees(struct merge_options *o,
 
                hashmap_free(&o->current_file_dir_set, 1);
 
-               if (clean < 0)
+               if (clean < 0) {
+                       unpack_trees_finish(o);
                        return clean;
+               }
        }
        else
                clean = 1;
 
-       /* Free the extra index left from git_merge_trees() */
-       /*
-        * FIXME: Need to also data allocated by setup_unpack_trees_porcelain()
-        * tucked away in o->unpack_opts.msgs, but the problem is that only
-        * half of it refers to dynamically allocated data, while the other
-        * half points at static strings.
-        */
-       discard_index(&o->orig_index);
+       unpack_trees_finish(o);
 
        if (o->call_depth && !(*result = write_tree_from_memory(o)))
                return -1;
-- 
2.17.0

Reply via email to