Re: [PATCH 08/23] builtin/ls-remote: fix leaking `pattern` strings

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Wed, Jul 31, 2024 at 12:35:29PM -0400, Taylor Blau wrote:
> 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@pks.im>
> > ---
> >  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.

Agreed, that reads way better. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]