Re: [PATCH 2/7] merge-ort: add a clear_internal_opts helper
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Dec 3, 2020, 17:00 UTC
- Message-ID
- <42a99014-573f-cc5d-ffa0-b22aec288425@gmail.com>
- In-Reply-To
- <5d73827b8d6c254f5bc4a99afa421ae34f07182e.1607011187.git.gitgitgadget@gmail.com>
On 12/3/2020 10:59 AM, Elijah Newren via GitGitGadget wrote:
Show 23 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> Move most of merge_finalize() into a new helper function,
> clear_internal_opts(). This is a step to facilitate recursive merges,
> as well as some future optimizations.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> merge-ort.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index b556897bc0..0654c76c8c 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -194,6 +194,29 @@ static void free_strmap_strings(struct strmap *map)
> }
> }
>
> +static void clear_internal_opts(struct merge_options_internal *opti,
> + int reinitialize)
> +{
> + assert(!reinitialize);I was first confused by this new assert, but you are essentially saying "this parameter doesn't do anything (yet)" which makes sense.
Show 17 quoted lines
> + > + /* > + * We marked opti->paths with strdup_strings = 0, so that we > + * wouldn't have to make another copy of the fullpath created by > + * make_traverse_path from setup_path_info(). But, now that we've > + * used it and have no other references to these strings, it is time > + * to deallocate them. > + */ > + free_strmap_strings(&opti->paths); > + strmap_clear(&opti->paths, 1); > + > + /* > + * All keys and values in opti->conflicted are a subset of those in > + * opti->paths. We don't want to deallocate anything twice, so we > + * don't free the keys and we pass 0 for free_values. > + */ > + strmap_clear(&opti->conflicted, 0);
...
Show 16 quoted lines
> - /* > - * We marked opti->paths with strdup_strings = 0, so that we > - * wouldn't have to make another copy of the fullpath created by > - * make_traverse_path from setup_path_info(). But, now that we've > - * used it and have no other references to these strings, it is time > - * to deallocate them. > - */ > - free_strmap_strings(&opti->paths); > - strmap_clear(&opti->paths, 1); > - > - /* > - * All keys and values in opti->conflicted are a subset of those in > - * opti->paths. We don't want to deallocate anything twice, so we > - * don't free the keys and we pass 0 for free_values. > - */ > - strmap_clear(&opti->conflicted, 0);
the rest of this is a clear code move.
Thanks, -Stolee