Message ID | patch-11.11-475b1196a93-20220713T131601Z-avarab@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | submodule--helper: fix memory leaks | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Fix leaks in the "reference" member of "struct module_clone_data" that > have been with us since 31224cbdc72 (clone: recursive and reference > option triggers submodule alternates, 2016-08-17) and > 8c8195e9c3e (submodule--helper: introduce add-clone subcommand, > 2021-07-10). > > Those commits added an xstrdup()'d member of the > STRING_LIST_INIT_NODUP'd "struct string_list". We need to free() > those, but not the ones we get from argv, let's make use of the "util" > member, if it has a pointer it's the pointer we'll need to free. Yes, this is a clear-cut case of "sometimes free()-able, sometimes not" (the others upthread were much less clear), so the approach sounds good. Well, we could make this always free()-able with some wacky OPT_STRING_LIST_DUP thing, but I don't think that idea is going anywhere ;) > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/submodule--helper.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index d9fab5d77da..966f543fbd9 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -1598,6 +1598,7 @@ struct module_clone_data { > static void module_clone_data_release(struct module_clone_data *cd) > { > free(cd->path_free); > + string_list_clear(&cd->reference, 1); > } > > struct submodule_alternate_setup { > @@ -1652,7 +1653,9 @@ static int add_possible_reference_from_superproject( > > sm_alternate = compute_alternate_path(sb.buf, &err); > if (sm_alternate) { > - string_list_append(sas->reference, xstrdup(sb.buf)); > + char *p = strbuf_detach(&sb, NULL); > + > + string_list_append(sas->reference, p)->util = p; > free(sm_alternate); > } else { > switch (sas->error_mode) { > @@ -3194,9 +3197,11 @@ static int add_submodule(const struct add_data *add_data) > clone_data.url = add_data->realrepo; > clone_data.quiet = add_data->quiet; > clone_data.progress = add_data->progress; > - if (add_data->reference_path) > - string_list_append(&clone_data.reference, > - xstrdup(add_data->reference_path)); > + if (add_data->reference_path) { > + char *p = xstrdup(add_data->reference_path); > + > + string_list_append(&clone_data.reference, p)->util = p; > + } > clone_data.dissociate = add_data->dissociate; > if (add_data->depth >= 0) > clone_data.depth = xstrfmt("%d", add_data->depth); > -- > 2.37.0.932.g7b7031e73bc Ok.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index d9fab5d77da..966f543fbd9 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1598,6 +1598,7 @@ struct module_clone_data { static void module_clone_data_release(struct module_clone_data *cd) { free(cd->path_free); + string_list_clear(&cd->reference, 1); } struct submodule_alternate_setup { @@ -1652,7 +1653,9 @@ static int add_possible_reference_from_superproject( sm_alternate = compute_alternate_path(sb.buf, &err); if (sm_alternate) { - string_list_append(sas->reference, xstrdup(sb.buf)); + char *p = strbuf_detach(&sb, NULL); + + string_list_append(sas->reference, p)->util = p; free(sm_alternate); } else { switch (sas->error_mode) { @@ -3194,9 +3197,11 @@ static int add_submodule(const struct add_data *add_data) clone_data.url = add_data->realrepo; clone_data.quiet = add_data->quiet; clone_data.progress = add_data->progress; - if (add_data->reference_path) - string_list_append(&clone_data.reference, - xstrdup(add_data->reference_path)); + if (add_data->reference_path) { + char *p = xstrdup(add_data->reference_path); + + string_list_append(&clone_data.reference, p)->util = p; + } clone_data.dissociate = add_data->dissociate; if (add_data->depth >= 0) clone_data.depth = xstrfmt("%d", add_data->depth);
Fix leaks in the "reference" member of "struct module_clone_data" that have been with us since 31224cbdc72 (clone: recursive and reference option triggers submodule alternates, 2016-08-17) and 8c8195e9c3e (submodule--helper: introduce add-clone subcommand, 2021-07-10). Those commits added an xstrdup()'d member of the STRING_LIST_INIT_NODUP'd "struct string_list". We need to free() those, but not the ones we get from argv, let's make use of the "util" member, if it has a pointer it's the pointer we'll need to free. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/submodule--helper.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)