From 05c4c1d165f1d67cc494403d794097526ab538c9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Dec 2024 13:13:40 +0100 Subject: [PATCH 1/3] sub-process: avoid leaking `cmd` In some instances (particularly the `read_object` hook), the `cmd` attribute is set to an `strdup()`ed value. This value needs to be released in the end! Since other users assign a non-`strdup()`ed value, be careful to add _another_ attribute (called `to_free`) that can hold a reference to such a string that needs to be released once the sub process is done. Signed-off-by: Johannes Schindelin --- sub-process.c | 9 ++++++++- sub-process.h | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/sub-process.c b/sub-process.c index 29a65f8aebbd9d..86a0d3084b75d9 100644 --- a/sub-process.c +++ b/sub-process.c @@ -63,6 +63,8 @@ void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) finish_command(&entry->process); hashmap_remove(hashmap, &entry->ent, NULL); + FREE_AND_NULL(entry->to_free); + entry->cmd = NULL; } static void subprocess_exit_handler(struct child_process *process) @@ -100,6 +102,7 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co process->trace2_child_class = "subprocess"; entry->cmd = process->args.v[0]; + entry->to_free = NULL; err = start_command(process); if (err) { @@ -145,11 +148,13 @@ int subprocess_start_strvec(struct hashmap *hashmap, process->trace2_child_class = "subprocess"; sq_quote_argv_pretty("ed, argv->v); - entry->cmd = strbuf_detach("ed, NULL); + entry->cmd = entry->to_free = strbuf_detach("ed, NULL); err = start_command(process); if (err) { error("cannot fork to run subprocess '%s'", entry->cmd); + FREE_AND_NULL(entry->to_free); + entry->cmd = NULL; return err; } @@ -158,6 +163,8 @@ int subprocess_start_strvec(struct hashmap *hashmap, err = startfn(entry); if (err) { error("initialization for subprocess '%s' failed", entry->cmd); + FREE_AND_NULL(entry->to_free); + entry->cmd = NULL; subprocess_stop(hashmap, entry); return err; } diff --git a/sub-process.h b/sub-process.h index 73cc536646df79..926d43ae2d2054 100644 --- a/sub-process.h +++ b/sub-process.h @@ -25,6 +25,12 @@ struct subprocess_entry { struct hashmap_entry ent; const char *cmd; + /** + * In case `cmd` is a `strdup()`ed value that needs to be released, + * you can assign the pointer to `to_free` so that `subprocess_stop()` + * will release it. + */ + char *to_free; struct child_process process; }; From ce632e94a99acbc566e5c1fb1262b7e8f6b1a7d3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Dec 2024 23:20:07 +0100 Subject: [PATCH 2/3] remote-curl: release filter options before re-setting them This fixes a leak that is not detected by Git's test suite (but by microsoft/git's). Signed-off-by: Johannes Schindelin --- remote-curl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/remote-curl.c b/remote-curl.c index b2898809bd9f6a..c2068eaf1ea180 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -212,6 +212,7 @@ static int set_option(const char *name, size_t namelen, const char *value) options.refetch = 1; return 0; } else if (!strncmp(name, "filter", namelen)) { + free(options.filter); options.filter = xstrdup(value); return 0; } else if (!strncmp(name, "object-format", namelen)) { From 2955dc8ecfa4c5eace39553e46679b996992e4a8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Dec 2024 23:22:02 +0100 Subject: [PATCH 3/3] transport: release object filter options This fixes a leak that is not detected by Git's own test suite (but by microsoft/git's, in the t9210-scalar.sh test). Signed-off-by: Johannes Schindelin --- transport-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/transport-helper.c b/transport-helper.c index 7513fd7eea05e4..5459010f44b54e 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -404,6 +404,7 @@ static int release_helper(struct transport *transport) free(data->import_marks); free(data->export_marks); res = disconnect_helper(transport); + list_objects_filter_release(&data->transport_options.filter_options); free(transport->data); return res; }