Re: [PATCH v2 3/4] Add 'promisor-remote' capability to protocol v2

[Patrick Steinhardt <ps@pks.im>]

On Tue, Sep 10, 2024 at 06:29:59PM +0200, Christian Couder wrote:
[snip]
> +static void filter_promisor_remote(struct repository *repo,
> +				   struct strvec *accepted,
> +				   const char *info)
> +{
> +	struct strbuf **remotes;
> +	char *accept_str;
> +	enum accept_promisor accept = ACCEPT_NONE;
> +
> +	if (!git_config_get_string("promisor.acceptfromserver", &accept_str)) {
> +		if (!accept_str || !*accept_str || !strcasecmp("None", accept_str))
> +			accept = ACCEPT_NONE;
> +		else if (!strcasecmp("All", accept_str))
> +			accept = ACCEPT_ALL;
> +		else
> +			warning(_("unknown '%s' value for '%s' config option"),
> +				accept_str, "promisor.acceptfromserver");
> +	}
> +
> +	if (accept == ACCEPT_NONE)
> +		return;

This code path is leaking memory because we don't free `accept_str`.
Once you reroll, I'd propose to have below patch on top to fix the leak.

Patrick

diff --git a/promisor-remote.c b/promisor-remote.c
index 06507b2ee1..0a4f7f1188 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -424,12 +424,12 @@ static void filter_promisor_remote(struct repository *repo,
 				   const char *info)
 {
 	struct strbuf **remotes;
-	char *accept_str;
+	const char *accept_str;
 	enum accept_promisor accept = ACCEPT_NONE;
 	struct strvec names = STRVEC_INIT;
 	struct strvec urls = STRVEC_INIT;
 
-	if (!git_config_get_string("promisor.acceptfromserver", &accept_str)) {
+	if (!git_config_get_string_tmp("promisor.acceptfromserver", &accept_str)) {
 		if (!accept_str || !*accept_str || !strcasecmp("None", accept_str))
 			accept = ACCEPT_NONE;
 		else if (!strcasecmp("KnownUrl", accept_str))
@@ -486,7 +486,6 @@ static void filter_promisor_remote(struct repository *repo,
 		free(decoded_url);
 	}
 
-	free(accept_str);
 	strvec_clear(&names);
 	strvec_clear(&urls);
 	strbuf_list_free(remotes);