Hi Elijah,

On Thu, 25 Jul 2019, Elijah Newren wrote:

> merge_options has several internal fields that should not be set or read
> by external callers.  This just complicates the API.  Move them into an
> opaque merge_options_internal struct that is defined only in
> merge-recursive.c and keep these out of merge-recursive.h.

This method requires an extra `malloc()`/`free()` pair, leaving room for
leaks.

I'd rather keep the definition of that struct in `merge-recursive.h`,
and make the `priv` field a full struct instead of a pointer. That would
save on that extra allocation.

Ciao,
Dscho

>
> Signed-off-by: Elijah Newren <new...@gmail.com>
> ---
>  merge-recursive.c | 185 +++++++++++++++++++++++++---------------------
>  merge-recursive.h |  17 ++---
>  2 files changed, 106 insertions(+), 96 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index a5049b06a3..c572d37b21 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -29,6 +29,15 @@
>  #include "revision.h"
>  #include "commit-reach.h"
>
> +struct merge_options_internal {
> +     int call_depth;
> +     int needed_rename_limit;
> +     struct hashmap current_file_dir_set;
> +     struct string_list df_conflict_file_set;
> +     struct unpack_trees_options unpack_opts;
> +     struct index_state orig_index;
> +};
> +
>  struct path_hashmap_entry {
>       struct hashmap_entry e;
>       char path[FLEX_ARRAY];
> @@ -309,7 +318,8 @@ static inline void setup_rename_conflict_info(enum 
> rename_type rename_type,
>
>  static int show(struct merge_options *opt, int v)
>  {
> -     return (!opt->call_depth && opt->verbosity >= v) || opt->verbosity >= 5;
> +     return (!opt->priv->call_depth && opt->verbosity >= v) ||
> +             opt->verbosity >= 5;
>  }
>
>  __attribute__((format (printf, 3, 4)))
> @@ -320,7 +330,7 @@ static void output(struct merge_options *opt, int v, 
> const char *fmt, ...)
>       if (!show(opt, v))
>               return;
>
> -     strbuf_addchars(&opt->obuf, ' ', opt->call_depth * 2);
> +     strbuf_addchars(&opt->obuf, ' ', opt->priv->call_depth * 2);
>
>       va_start(ap, fmt);
>       strbuf_vaddf(&opt->obuf, fmt, ap);
> @@ -335,7 +345,7 @@ static void output_commit_title(struct merge_options 
> *opt, struct commit *commit
>  {
>       struct merge_remote_desc *desc;
>
> -     strbuf_addchars(&opt->obuf, ' ', opt->call_depth * 2);
> +     strbuf_addchars(&opt->obuf, ' ', opt->priv->call_depth * 2);
>       desc = merge_remote_util(commit);
>       if (desc)
>               strbuf_addf(&opt->obuf, "virtual %s\n", desc->name);
> @@ -403,43 +413,43 @@ static int unpack_trees_start(struct merge_options *opt,
>       struct tree_desc t[3];
>       struct index_state tmp_index = { NULL };
>
> -     memset(&opt->unpack_opts, 0, sizeof(opt->unpack_opts));
> -     if (opt->call_depth)
> -             opt->unpack_opts.index_only = 1;
> +     memset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));
> +     if (opt->priv->call_depth)
> +             opt->priv->unpack_opts.index_only = 1;
>       else
> -             opt->unpack_opts.update = 1;
> -     opt->unpack_opts.merge = 1;
> -     opt->unpack_opts.head_idx = 2;
> -     opt->unpack_opts.fn = threeway_merge;
> -     opt->unpack_opts.src_index = opt->repo->index;
> -     opt->unpack_opts.dst_index = &tmp_index;
> -     opt->unpack_opts.aggressive = !merge_detect_rename(opt);
> -     setup_unpack_trees_porcelain(&opt->unpack_opts, "merge");
> +             opt->priv->unpack_opts.update = 1;
> +     opt->priv->unpack_opts.merge = 1;
> +     opt->priv->unpack_opts.head_idx = 2;
> +     opt->priv->unpack_opts.fn = threeway_merge;
> +     opt->priv->unpack_opts.src_index = opt->repo->index;
> +     opt->priv->unpack_opts.dst_index = &tmp_index;
> +     opt->priv->unpack_opts.aggressive = !merge_detect_rename(opt);
> +     setup_unpack_trees_porcelain(&opt->priv->unpack_opts, "merge");
>
>       init_tree_desc_from_tree(t+0, common);
>       init_tree_desc_from_tree(t+1, head);
>       init_tree_desc_from_tree(t+2, merge);
>
> -     rc = unpack_trees(3, t, &opt->unpack_opts);
> +     rc = unpack_trees(3, t, &opt->priv->unpack_opts);
>       cache_tree_free(&opt->repo->index->cache_tree);
>
>       /*
> -      * Update opt->repo->index to match the new results, AFTER saving a copy
> -      * in opt->orig_index.  Update src_index to point to the saved copy.
> -      * (verify_uptodate() checks src_index, and the original index is
> -      * the one that had the necessary modification timestamps.)
> +      * Update opt->repo->index to match the new results, AFTER saving a
> +      * copy in opt->priv->orig_index.  Update src_index to point to the
> +      * saved copy.  (verify_uptodate() checks src_index, and the original
> +      * index is the one that had the necessary modification timestamps.)
>        */
> -     opt->orig_index = *opt->repo->index;
> +     opt->priv->orig_index = *opt->repo->index;
>       *opt->repo->index = tmp_index;
> -     opt->unpack_opts.src_index = &opt->orig_index;
> +     opt->priv->unpack_opts.src_index = &opt->priv->orig_index;
>
>       return rc;
>  }
>
>  static void unpack_trees_finish(struct merge_options *opt)
>  {
> -     discard_index(&opt->orig_index);
> -     clear_unpack_trees_porcelain(&opt->unpack_opts);
> +     discard_index(&opt->priv->orig_index);
> +     clear_unpack_trees_porcelain(&opt->priv->unpack_opts);
>  }
>
>  static int save_files_dirs(const struct object_id *oid,
> @@ -454,7 +464,7 @@ static int save_files_dirs(const struct object_id *oid,
>
>       FLEX_ALLOC_MEM(entry, path, base->buf, base->len);
>       hashmap_entry_init(entry, path_hash(entry->path));
> -     hashmap_add(&opt->current_file_dir_set, entry);
> +     hashmap_add(&opt->priv->current_file_dir_set, entry);
>
>       strbuf_setlen(base, baselen);
>       return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
> @@ -585,7 +595,7 @@ static void record_df_conflict_files(struct merge_options 
> *opt,
>        * If we're merging merge-bases, we don't want to bother with
>        * any working directory changes.
>        */
> -     if (opt->call_depth)
> +     if (opt->priv->call_depth)
>               return;
>
>       /* Ensure D/F conflicts are adjacent in the entries list. */
> @@ -597,7 +607,7 @@ static void record_df_conflict_files(struct merge_options 
> *opt,
>       df_sorted_entries.cmp = string_list_df_name_compare;
>       string_list_sort(&df_sorted_entries);
>
> -     string_list_clear(&opt->df_conflict_file_set, 1);
> +     string_list_clear(&opt->priv->df_conflict_file_set, 1);
>       for (i = 0; i < df_sorted_entries.nr; i++) {
>               const char *path = df_sorted_entries.items[i].string;
>               int len = strlen(path);
> @@ -613,7 +623,7 @@ static void record_df_conflict_files(struct merge_options 
> *opt,
>                   len > last_len &&
>                   memcmp(path, last_file, last_len) == 0 &&
>                   path[last_len] == '/') {
> -                     string_list_insert(&opt->df_conflict_file_set, 
> last_file);
> +                     string_list_insert(&opt->priv->df_conflict_file_set, 
> last_file);
>               }
>
>               /*
> @@ -680,8 +690,8 @@ static void update_entry(struct stage_data *entry,
>  static int remove_file(struct merge_options *opt, int clean,
>                      const char *path, int no_wd)
>  {
> -     int update_cache = opt->call_depth || clean;
> -     int update_working_directory = !opt->call_depth && !no_wd;
> +     int update_cache = opt->priv->call_depth || clean;
> +     int update_working_directory = !opt->priv->call_depth && !no_wd;
>
>       if (update_cache) {
>               if (remove_file_from_index(opt->repo->index, path))
> @@ -724,16 +734,16 @@ static char *unique_path(struct merge_options *opt,
>       add_flattened_path(&newpath, branch);
>
>       base_len = newpath.len;
> -     while (hashmap_get_from_hash(&opt->current_file_dir_set,
> +     while (hashmap_get_from_hash(&opt->priv->current_file_dir_set,
>                                    path_hash(newpath.buf), newpath.buf) ||
> -            (!opt->call_depth && file_exists(newpath.buf))) {
> +            (!opt->priv->call_depth && file_exists(newpath.buf))) {
>               strbuf_setlen(&newpath, base_len);
>               strbuf_addf(&newpath, "_%d", suffix++);
>       }
>
>       FLEX_ALLOC_MEM(entry, path, newpath.buf, newpath.len);
>       hashmap_entry_init(entry, path_hash(entry->path));
> -     hashmap_add(&opt->current_file_dir_set, entry);
> +     hashmap_add(&opt->priv->current_file_dir_set, entry);
>       return strbuf_detach(&newpath, NULL);
>  }
>
> @@ -775,7 +785,7 @@ static int dir_in_way(struct index_state *istate, const 
> char *path,
>  static int was_tracked_and_matches(struct merge_options *opt, const char 
> *path,
>                                  const struct diff_filespec *blob)
>  {
> -     int pos = index_name_pos(&opt->orig_index, path, strlen(path));
> +     int pos = index_name_pos(&opt->priv->orig_index, path, strlen(path));
>       struct cache_entry *ce;
>
>       if (0 > pos)
> @@ -783,7 +793,7 @@ static int was_tracked_and_matches(struct merge_options 
> *opt, const char *path,
>               return 0;
>
>       /* See if the file we were tracking before matches */
> -     ce = opt->orig_index.cache[pos];
> +     ce = opt->priv->orig_index.cache[pos];
>       return (oid_eq(&ce->oid, &blob->oid) && ce->ce_mode == blob->mode);
>  }
>
> @@ -792,7 +802,7 @@ static int was_tracked_and_matches(struct merge_options 
> *opt, const char *path,
>   */
>  static int was_tracked(struct merge_options *opt, const char *path)
>  {
> -     int pos = index_name_pos(&opt->orig_index, path, strlen(path));
> +     int pos = index_name_pos(&opt->priv->orig_index, path, strlen(path));
>
>       if (0 <= pos)
>               /* we were tracking this path before the merge */
> @@ -849,12 +859,12 @@ static int was_dirty(struct merge_options *opt, const 
> char *path)
>       struct cache_entry *ce;
>       int dirty = 1;
>
> -     if (opt->call_depth || !was_tracked(opt, path))
> +     if (opt->priv->call_depth || !was_tracked(opt, path))
>               return !dirty;
>
> -     ce = index_file_exists(opt->unpack_opts.src_index,
> +     ce = index_file_exists(opt->priv->unpack_opts.src_index,
>                              path, strlen(path), ignore_case);
> -     dirty = verify_uptodate(ce, &opt->unpack_opts) != 0;
> +     dirty = verify_uptodate(ce, &opt->priv->unpack_opts) != 0;
>       return dirty;
>  }
>
> @@ -864,8 +874,8 @@ static int make_room_for_path(struct merge_options *opt, 
> const char *path)
>       const char *msg = _("failed to create path '%s'%s");
>
>       /* Unlink any D/F conflict files that are in the way */
> -     for (i = 0; i < opt->df_conflict_file_set.nr; i++) {
> -             const char *df_path = opt->df_conflict_file_set.items[i].string;
> +     for (i = 0; i < opt->priv->df_conflict_file_set.nr; i++) {
> +             const char *df_path = 
> opt->priv->df_conflict_file_set.items[i].string;
>               size_t pathlen = strlen(path);
>               size_t df_pathlen = strlen(df_path);
>               if (df_pathlen < pathlen &&
> @@ -875,7 +885,7 @@ static int make_room_for_path(struct merge_options *opt, 
> const char *path)
>                              _("Removing %s to make room for subdirectory\n"),
>                              df_path);
>                       unlink(df_path);
> -                     
> unsorted_string_list_delete_item(&opt->df_conflict_file_set,
> +                     
> unsorted_string_list_delete_item(&opt->priv->df_conflict_file_set,
>                                                        i, 0);
>                       break;
>               }
> @@ -916,7 +926,7 @@ static int update_file_flags(struct merge_options *opt,
>  {
>       int ret = 0;
>
> -     if (opt->call_depth)
> +     if (opt->priv->call_depth)
>               update_wd = 0;
>
>       if (update_wd) {
> @@ -1001,7 +1011,7 @@ static int update_file(struct merge_options *opt,
>                      const char *path)
>  {
>       return update_file_flags(opt, contents, path,
> -                              opt->call_depth || clean, !opt->call_depth);
> +                              opt->priv->call_depth || clean, 
> !opt->priv->call_depth);
>  }
>
>  /* Low level file merging, update and removal */
> @@ -1030,7 +1040,7 @@ static int merge_3way(struct merge_options *opt,
>       ll_opts.extra_marker_size = extra_marker_size;
>       ll_opts.xdl_opts = opt->xdl_opts;
>
> -     if (opt->call_depth) {
> +     if (opt->priv->call_depth) {
>               ll_opts.virtual_ancestor = 1;
>               ll_opts.variant = 0;
>       } else {
> @@ -1164,7 +1174,7 @@ static int merge_submodule(struct merge_options *opt,
>       struct object_array merges;
>
>       int i;
> -     int search = !opt->call_depth;
> +     int search = !opt->priv->call_depth;
>
>       /* store a in result in case we fail */
>       oidcpy(result, a);
> @@ -1385,7 +1395,7 @@ static int handle_rename_via_dir(struct merge_options 
> *opt,
>       int mark_conflicted = (opt->detect_directory_renames == 1);
>       assert(ren->dir_rename_original_dest);
>
> -     if (!opt->call_depth && would_lose_untracked(opt, dest->path)) {
> +     if (!opt->priv->call_depth && would_lose_untracked(opt, dest->path)) {
>               mark_conflicted = 1;
>               file_path = unique_path(opt, dest->path, ren->branch);
>               output(opt, 1, _("Error: Refusing to lose untracked file at %s; 
> "
> @@ -1428,12 +1438,12 @@ static int handle_change_delete(struct merge_options 
> *opt,
>       const char *update_path = path;
>       int ret = 0;
>
> -     if (dir_in_way(opt->repo->index, path, !opt->call_depth, 0) ||
> -         (!opt->call_depth && would_lose_untracked(opt, path))) {
> +     if (dir_in_way(opt->repo->index, path, !opt->priv->call_depth, 0) ||
> +         (!opt->priv->call_depth && would_lose_untracked(opt, path))) {
>               update_path = alt_path = unique_path(opt, path, change_branch);
>       }
>
> -     if (opt->call_depth) {
> +     if (opt->priv->call_depth) {
>               /*
>                * We cannot arbitrarily accept either a_sha or b_sha as
>                * correct; since there is no true "middle point" between
> @@ -1508,14 +1518,14 @@ static int handle_rename_delete(struct merge_options 
> *opt,
>                                    opt->branch2 : opt->branch1);
>
>       if (handle_change_delete(opt,
> -                              opt->call_depth ? orig->path : dest->path,
> -                              opt->call_depth ? NULL : orig->path,
> +                              opt->priv->call_depth ? orig->path : 
> dest->path,
> +                              opt->priv->call_depth ? NULL : orig->path,
>                                orig, dest,
>                                rename_branch, delete_branch,
>                                _("rename"), _("renamed")))
>               return -1;
>
> -     if (opt->call_depth)
> +     if (opt->priv->call_depth)
>               return remove_file_from_index(opt->repo->index, dest->path);
>       else
>               return update_stages(opt, dest->path, NULL,
> @@ -1552,7 +1562,7 @@ static int handle_file_collision(struct merge_options 
> *opt,
>       /*
>        * In the recursive case, we just opt to undo renames
>        */
> -     if (opt->call_depth && (prev_path1 || prev_path2)) {
> +     if (opt->priv->call_depth && (prev_path1 || prev_path2)) {
>               /* Put first file (a->oid, a->mode) in its original spot */
>               if (prev_path1) {
>                       if (update_file(opt, 1, a, prev_path1))
> @@ -1581,10 +1591,10 @@ static int handle_file_collision(struct merge_options 
> *opt,
>       /* Remove rename sources if rename/add or rename/rename(2to1) */
>       if (prev_path1)
>               remove_file(opt, 1, prev_path1,
> -                         opt->call_depth || would_lose_untracked(opt, 
> prev_path1));
> +                         opt->priv->call_depth || would_lose_untracked(opt, 
> prev_path1));
>       if (prev_path2)
>               remove_file(opt, 1, prev_path2,
> -                         opt->call_depth || would_lose_untracked(opt, 
> prev_path2));
> +                         opt->priv->call_depth || would_lose_untracked(opt, 
> prev_path2));
>
>       /*
>        * Remove the collision path, if it wouldn't cause dirty contents
> @@ -1626,12 +1636,12 @@ static int handle_file_collision(struct merge_options 
> *opt,
>       null.mode = 0;
>
>       if (merge_mode_and_contents(opt, &null, a, b, collide_path,
> -                                 branch1, branch2, opt->call_depth * 2, 
> &mfi))
> +                                 branch1, branch2, opt->priv->call_depth * 
> 2, &mfi))
>               return -1;
>       mfi.clean &= !alt_path;
>       if (update_file(opt, mfi.clean, &mfi.blob, update_path))
>               return -1;
> -     if (!mfi.clean && !opt->call_depth &&
> +     if (!mfi.clean && !opt->priv->call_depth &&
>           update_stages(opt, collide_path, NULL, a, b))
>               return -1;
>       free(alt_path);
> @@ -1671,7 +1681,7 @@ static int handle_rename_add(struct merge_options *opt,
>                                   &ci->ren1->src_entry->stages[other_stage],
>                                   prev_path_desc,
>                                   opt->branch1, opt->branch2,
> -                                 1 + opt->call_depth * 2, &mfi))
> +                                 1 + opt->priv->call_depth * 2, &mfi))
>               return -1;
>       free(prev_path_desc);
>
> @@ -1689,7 +1699,7 @@ static char *find_path_for_conflict(struct 
> merge_options *opt,
>                                   const char *branch2)
>  {
>       char *new_path = NULL;
> -     if (dir_in_way(opt->repo->index, path, !opt->call_depth, 0)) {
> +     if (dir_in_way(opt->repo->index, path, !opt->priv->call_depth, 0)) {
>               new_path = unique_path(opt, path, branch1);
>               output(opt, 1, _("%s is a directory in %s adding "
>                              "as %s instead"),
> @@ -1720,17 +1730,17 @@ static int handle_rename_rename_1to2(struct 
> merge_options *opt,
>              "rename \"%s\"->\"%s\" in \"%s\"%s"),
>              o->path, a->path, ci->ren1->branch,
>              o->path, b->path, ci->ren2->branch,
> -            opt->call_depth ? _(" (left unresolved)") : "");
> +            opt->priv->call_depth ? _(" (left unresolved)") : "");
>
>       path_desc = xstrfmt("%s and %s, both renamed from %s",
>                           a->path, b->path, o->path);
>       if (merge_mode_and_contents(opt, o, a, b, path_desc,
>                                   ci->ren1->branch, ci->ren2->branch,
> -                                 opt->call_depth * 2, &mfi))
> +                                 opt->priv->call_depth * 2, &mfi))
>               return -1;
>       free(path_desc);
>
> -     if (opt->call_depth) {
> +     if (opt->priv->call_depth) {
>               /*
>                * FIXME: For rename/add-source conflicts (if we could detect
>                * such), this is wrong.  We should instead find a unique
> @@ -1845,12 +1855,12 @@ static int handle_rename_rename_2to1(struct 
> merge_options *opt,
>                                   &ci->ren1->src_entry->stages[ostage1],
>                                   path_side_1_desc,
>                                   opt->branch1, opt->branch2,
> -                                 1 + opt->call_depth * 2, &mfi_c1) ||
> +                                 1 + opt->priv->call_depth * 2, &mfi_c1) ||
>           merge_mode_and_contents(opt, b,
>                                   &ci->ren2->src_entry->stages[ostage2],
>                                   c2, path_side_2_desc,
>                                   opt->branch1, opt->branch2,
> -                                 1 + opt->call_depth * 2, &mfi_c2))
> +                                 1 + opt->priv->call_depth * 2, &mfi_c2))
>               return -1;
>       free(path_side_1_desc);
>       free(path_side_2_desc);
> @@ -1891,8 +1901,8 @@ static struct diff_queue_struct *get_diffpairs(struct 
> merge_options *opt,
>       diff_setup_done(&opts);
>       diff_tree_oid(&o_tree->object.oid, &tree->object.oid, "", &opts);
>       diffcore_std(&opts);
> -     if (opts.needed_rename_limit > opt->needed_rename_limit)
> -             opt->needed_rename_limit = opts.needed_rename_limit;
> +     if (opts.needed_rename_limit > opt->priv->needed_rename_limit)
> +             opt->priv->needed_rename_limit = opts.needed_rename_limit;
>
>       ret = xmalloc(sizeof(*ret));
>       *ret = diff_queued_diff;
> @@ -3022,13 +3032,13 @@ static int handle_content_merge(struct 
> merge_file_info *mfi,
>               reason = _("add/add");
>
>       assert(o->path && a->path && b->path);
> -     if (ci && dir_in_way(opt->repo->index, path, !opt->call_depth,
> +     if (ci && dir_in_way(opt->repo->index, path, !opt->priv->call_depth,
>                            S_ISGITLINK(ci->ren1->pair->two->mode)))
>               df_conflict_remains = 1;
>
>       if (merge_mode_and_contents(opt, o, a, b, path,
>                                   opt->branch1, opt->branch2,
> -                                 opt->call_depth * 2, mfi))
> +                                 opt->priv->call_depth * 2, mfi))
>               return -1;
>
>       /*
> @@ -3044,7 +3054,7 @@ static int handle_content_merge(struct merge_file_info 
> *mfi,
>
>               output(opt, 3, _("Skipped %s (merged same as existing)"), path);
>               if (add_cacheinfo(opt, &mfi->blob, path,
> -                               0, (!opt->call_depth && !is_dirty), 0))
> +                               0, (!opt->priv->call_depth && !is_dirty), 0))
>                       return -1;
>               /*
>                * However, add_cacheinfo() will delete the old cache entry
> @@ -3052,8 +3062,8 @@ static int handle_content_merge(struct merge_file_info 
> *mfi,
>                * flag to avoid making the file appear as if it were
>                * deleted by the user.
>                */
> -             pos = index_name_pos(&opt->orig_index, path, strlen(path));
> -             ce = opt->orig_index.cache[pos];
> +             pos = index_name_pos(&opt->priv->orig_index, path, 
> strlen(path));
> +             ce = opt->priv->orig_index.cache[pos];
>               if (ce_skip_worktree(ce)) {
>                       pos = index_name_pos(opt->repo->index, path, 
> strlen(path));
>                       ce = opt->repo->index->cache[pos];
> @@ -3074,7 +3084,7 @@ static int handle_content_merge(struct merge_file_info 
> *mfi,
>
>       if (df_conflict_remains || is_dirty) {
>               char *new_path;
> -             if (opt->call_depth) {
> +             if (opt->priv->call_depth) {
>                       remove_file_from_index(opt->repo->index, path);
>               } else {
>                       if (!mfi->clean) {
> @@ -3332,7 +3342,7 @@ static int process_entry(struct merge_options *opt,
>                       conf = _("directory/file");
>               }
>               if (dir_in_way(opt->repo->index, path,
> -                            !opt->call_depth && !S_ISGITLINK(a->mode),
> +                            !opt->priv->call_depth && !S_ISGITLINK(a->mode),
>                              0)) {
>                       char *new_path = unique_path(opt, path, add_branch);
>                       clean_merge = 0;
> @@ -3341,7 +3351,7 @@ static int process_entry(struct merge_options *opt,
>                              conf, path, other_branch, path, new_path);
>                       if (update_file(opt, 0, contents, new_path))
>                               clean_merge = -1;
> -                     else if (opt->call_depth)
> +                     else if (opt->priv->call_depth)
>                               remove_file_from_index(opt->repo->index, path);
>                       free(new_path);
>               } else {
> @@ -3406,7 +3416,7 @@ static int merge_trees_internal(struct merge_options 
> *opt,
>       code = unpack_trees_start(opt, merge_base, head, merge);
>
>       if (code != 0) {
> -             if (show(opt, 4) || opt->call_depth)
> +             if (show(opt, 4) || opt->priv->call_depth)
>                       err(opt, _("merging of trees %s and %s failed"),
>                           oid_to_hex(&head->object.oid),
>                           oid_to_hex(&merge->object.oid));
> @@ -3425,7 +3435,7 @@ static int merge_trees_internal(struct merge_options 
> *opt,
>                * opposed to decaring a local hashmap is for convenience
>                * so that we don't have to pass it to around.
>                */
> -             hashmap_init(&opt->current_file_dir_set, path_hashmap_cmp,
> +             hashmap_init(&opt->priv->current_file_dir_set, path_hashmap_cmp,
>                            NULL, 512);
>               get_files_dirs(opt, head);
>               get_files_dirs(opt, merge);
> @@ -3462,7 +3472,7 @@ static int merge_trees_internal(struct merge_options 
> *opt,
>               string_list_clear(entries, 1);
>               free(entries);
>
> -             hashmap_free(&opt->current_file_dir_set, 1);
> +             hashmap_free(&opt->priv->current_file_dir_set, 1);
>
>               if (clean < 0) {
>                       unpack_trees_finish(opt);
> @@ -3474,7 +3484,8 @@ static int merge_trees_internal(struct merge_options 
> *opt,
>
>       unpack_trees_finish(opt);
>
> -     if (opt->call_depth && !(*result = write_tree_from_memory(opt->repo)))
> +     if (opt->priv->call_depth &&
> +         !(*result = write_tree_from_memory(opt->repo)))
>               return -1;
>
>       return clean;
> @@ -3538,7 +3549,7 @@ static int merge_recursive_internal(struct 
> merge_options *opt,
>
>       for (iter = merge_bases; iter; iter = iter->next) {
>               const char *saved_b1, *saved_b2;
> -             opt->call_depth++;
> +             opt->priv->call_depth++;
>               /*
>                * When the merge fails, the result contains files
>                * with conflict markers. The cleanness flag is
> @@ -3557,14 +3568,14 @@ static int merge_recursive_internal(struct 
> merge_options *opt,
>                       return -1;
>               opt->branch1 = saved_b1;
>               opt->branch2 = saved_b2;
> -             opt->call_depth--;
> +             opt->priv->call_depth--;
>
>               if (!merged_merge_bases)
>                       return err(opt, _("merge returned no commit"));
>       }
>
>       discard_index(opt->repo->index);
> -     if (!opt->call_depth)
> +     if (!opt->priv->call_depth)
>               repo_read_index(opt->repo);
>
>       opt->ancestor = "merged common ancestors";
> @@ -3579,14 +3590,14 @@ static int merge_recursive_internal(struct 
> merge_options *opt,
>               return clean;
>       }
>
> -     if (opt->call_depth) {
> +     if (opt->priv->call_depth) {
>               *result = make_virtual_commit(opt->repo, result_tree,
>                                             "merged tree");
>               commit_list_insert(h1, &(*result)->parents);
>               commit_list_insert(h2, &(*result)->parents->next);
>       }
>       flush_output(opt);
> -     if (!opt->call_depth && opt->buffer_output < 2)
> +     if (!opt->priv->call_depth && opt->buffer_output < 2)
>               strbuf_release(&opt->obuf);
>       return clean;
>  }
> @@ -3603,6 +3614,8 @@ static int merge_start(struct merge_options *opt, 
> struct tree *head)
>               return -1;
>       }
>
> +     opt->priv = xcalloc(1, sizeof(*opt->priv));
> +     string_list_init(&opt->priv->df_conflict_file_set, 1);
>       return 0;
>  }
>
> @@ -3610,7 +3623,9 @@ static void merge_finalize(struct merge_options *opt)
>  {
>       if (show(opt, 2))
>               diff_warn_rename_limit("merge.renamelimit",
> -                                    opt->needed_rename_limit, 0);
> +                                    opt->priv->needed_rename_limit, 0);
> +     free(opt->priv);
> +     opt->priv = NULL;
>  }
>
>  int merge_trees(struct merge_options *opt,
> @@ -3748,8 +3763,6 @@ void init_merge_options(struct merge_options *opt,
>
>       opt->renormalize = 0;
>
> -     string_list_init(&opt->df_conflict_file_set, 1);
> -
>       merge_recursive_config(opt);
>       merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
>       if (merge_verbosity)
> diff --git a/merge-recursive.h b/merge-recursive.h
> index d57fce0daa..a1bb29dc33 100644
> --- a/merge-recursive.h
> +++ b/merge-recursive.h
> @@ -1,13 +1,15 @@
>  #ifndef MERGE_RECURSIVE_H
>  #define MERGE_RECURSIVE_H
>
> -#include "string-list.h"
> -#include "unpack-trees.h"
> +#include "strbuf.h"
>
>  struct commit;
> -
> +struct commit_list;
> +struct object_id;
>  struct repository;
> +struct tree;
>
> +struct merge_options_internal;
>  struct merge_options {
>       struct repository *repo;
>
> @@ -40,13 +42,8 @@ struct merge_options {
>       const char *subtree_shift;
>       unsigned renormalize : 1;
>
> -     /* internal fields used by the implementation (do NOT set these) */
> -     int call_depth;
> -     int needed_rename_limit;
> -     struct hashmap current_file_dir_set;
> -     struct string_list df_conflict_file_set;
> -     struct unpack_trees_options unpack_opts;
> -     struct index_state orig_index;
> +     /* internal fields used by the implementation */
> +     struct merge_options_internal *priv;
>  };
>
>  void init_merge_options(struct merge_options *opt, struct repository *repo);
> --
> 2.22.0.559.g28a8880890.dirty
>
>

Reply via email to