On Fri, Jul 26, 2024 at 02:15:24PM +0200, Patrick Steinhardt wrote:
> Users can pass patterns to git-ls-remote(1), which allows them to filter
> the list of printed references. We assemble those patterns into an array
> and prefix them with "*/", but never free either the array nor the
> allocated strings.
>
> Fix those leaks.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> builtin/ls-remote.c | 11 +++++++----
> t/t5535-fetch-push-symref.sh | 1 +
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index debf2d4f88..500f76fe4c 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> int status = 0;
> int show_symref_target = 0;
> const char *uploadpack = NULL;
> - const char **pattern = NULL;
> + char **pattern = NULL;
> struct transport_ls_refs_options transport_options =
> TRANSPORT_LS_REFS_OPTIONS_INIT;
> int i;
> @@ -96,9 +96,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> if (argc > 1) {
> int i;
> CALLOC_ARRAY(pattern, argc);
> - for (i = 1; i < argc; i++) {
> + for (i = 1; i < argc; i++)
> pattern[i - 1] = xstrfmt("*/%s", argv[i]);
> - }
Instead of xstrfmt()-ing these strings directly, it may be more
ergonomic to use a strvec here and call strvec_pushf(). That would save
you explicitly CALLOC'ing the array, and would also save you the
explicit free() on each element of pattern below.
> }
>
> if (flags & REF_TAGS)
> @@ -136,7 +135,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> struct ref_array_item *item;
> if (!check_ref_type(ref, flags))
> continue;
> - if (!tail_match(pattern, ref->name))
> + if (!tail_match((const char **) pattern, ref->name))
You could also drop the const cast here as well. The resulting patch on
top of this one simplifies things a bit:
--- 8< ---
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 500f76fe4c..b59ac98d81 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
int status = 0;
int show_symref_target = 0;
const char *uploadpack = NULL;
- char **pattern = NULL;
+ struct strvec pattern = STRVEC_INIT;
struct transport_ls_refs_options transport_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
int i;
@@ -93,12 +93,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
packet_trace_identity("ls-remote");
- if (argc > 1) {
- int i;
- CALLOC_ARRAY(pattern, argc);
- for (i = 1; i < argc; i++)
- pattern[i - 1] = xstrfmt("*/%s", argv[i]);
- }
+ for (int i = 1; i < argc; i++)
+ strvec_pushf(&pattern, "*/%s", argv[i]);
if (flags & REF_TAGS)
strvec_push(&transport_options.ref_prefixes, "refs/tags/");
@@ -135,7 +131,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
struct ref_array_item *item;
if (!check_ref_type(ref, flags))
continue;
- if (!tail_match((const char **) pattern, ref->name))
+ if (!tail_match(pattern.v, ref->name))
continue;
item = ref_array_push(&ref_array, ref->name, &ref->old_oid);
item->symref = xstrdup_or_null(ref->symref);
@@ -158,8 +154,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
status = 1;
transport_ls_refs_options_release(&transport_options);
- for (i = 1; i < argc; i++)
- free(pattern[i - 1]);
- free(pattern);
+ strvec_clear(&pattern);
return status;
}
--- >8 ---
Thanks,
Taylor
