| Message ID | 5d73827b8d6c254f5bc4a99afa421ae34f07182e.1607011187.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | merge-ort: some groundwork for further implementation | expand |
On 12/3/2020 10:59 AM, Elijah Newren via GitGitGadget wrote: > 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. > + > + /* > + * 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); ... > - /* > - * 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
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); + + /* + * 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); +} + static int err(struct merge_options *opt, const char *err, ...) { va_list params; @@ -1132,22 +1155,7 @@ void merge_finalize(struct merge_options *opt, assert(opt->priv == NULL); - /* - * 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); + clear_internal_opts(opti, 0); FREE_AND_NULL(opti); }