Elijah Newren <newren@xxxxxxxxx> writes:
> +struct rename_info {
> + struct string_list *head_renames;
> + struct string_list *merge_renames;
> +};
This type is added in order to allow the caller and the helper to
communicate the findings in a single logical structure, instead of
having to pass them as separate parameters, etc. If we anticipate
that the information that needs to be passed will grow richer in
later steps (or a follow-up series), such encapsulation makes a lot
of sence.
> +static struct rename_info *handle_renames(struct merge_options *o,
> + struct tree *common,
> + struct tree *head,
> + struct tree *merge,
> + struct string_list *entries,
> + int *clean)
> +{
> + struct rename_info *rei = xcalloc(1, sizeof(struct rename_info));
I however notice that there is only one caller of this helper at
this step, and also at the end of this series. I suspect that it
would probably be a better design to make "clean" the return value
of this helper, and instead have the caller pass an uninitialised
rename_info structure on its stack by address to be filled by the
helper.
> + rei->head_renames = get_renames(o, head, common, head, merge, entries);
> + rei->merge_renames = get_renames(o, merge, common, head, merge, entries);
> + *clean = process_renames(o, rei->head_renames, rei->merge_renames);
> +
> + return rei;
> +}
> +
> +static void cleanup_renames(struct rename_info *re_info)
> +{
> + string_list_clear(re_info->head_renames, 0);
> + string_list_clear(re_info->merge_renames, 0);
> +
> + free(re_info->head_renames);
> + free(re_info->merge_renames);
> +
> + free(re_info);
> +}
> static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)
> {
> return (is_null_oid(oid) || mode == 0) ? NULL: (struct object_id *)oid;
> @@ -1989,7 +2021,8 @@ int merge_trees(struct merge_options *o,
> }
>
> if (unmerged_cache()) {
> - struct string_list *entries, *re_head, *re_merge;
> + struct string_list *entries;
> + struct rename_info *re_info;
> int i;
> /*
> * Only need the hashmap while processing entries, so
> @@ -2003,9 +2036,7 @@ int merge_trees(struct merge_options *o,
> get_files_dirs(o, merge);
>
> entries = get_unmerged();
> - re_head = get_renames(o, head, common, head, merge, entries);
> - re_merge = get_renames(o, merge, common, head, merge, entries);
> - clean = process_renames(o, re_head, re_merge);
> + re_info = handle_renames(o, common, head, merge, entries, &clean);
> record_df_conflict_files(o, entries);
> if (clean < 0)
> goto cleanup;
> @@ -2030,16 +2061,13 @@ int merge_trees(struct merge_options *o,
> }
>
> cleanup:
> - string_list_clear(re_merge, 0);
> - string_list_clear(re_head, 0);
> + cleanup_renames(re_info);
> +
> string_list_clear(entries, 1);
> + free(entries);
>
> hashmap_free(&o->current_file_dir_set, 1);
>
> - free(re_merge);
> - free(re_head);
> - free(entries);
> -
> if (clean < 0)
> return clean;
> }
