diff mbox series

[02/22] git: fix leaking system paths

Message ID 9574995a246d96b90f03827bf0ba591593d6c4d9.1722933642.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Memory leak fixes (pt.4) | expand

Commit Message

Patrick Steinhardt Aug. 6, 2024, 8:59 a.m. UTC
Git has some flags to make it output system paths as they have been
compiled into Git. This is done by calling `system_path()`, which
returns an allocated string. This string isn't ever free'd though,
creating a memory leak.

Plug those leaks. While they are surfaced by t0211, there are more
memory leaks looming exposed by that test suite and it thus does not yet
pass with the memory leak checker enabled.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 git.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

James Liu Aug. 7, 2024, 4:02 a.m. UTC | #1
On Tue Aug 6, 2024 at 6:59 PM AEST, Patrick Steinhardt wrote:
> Git has some flags to make it output system paths as they have been
> compiled into Git. This is done by calling `system_path()`, which
> returns an allocated string. This string isn't ever free'd though,
> creating a memory leak.
>
> Plug those leaks. While they are surfaced by t0211, there are more
> memory leaks looming exposed by that test suite and it thus does not yet
> pass with the memory leak checker enabled.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  git.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/git.c b/git.c
> index e35af9b0e5..5eab88b472 100644
> --- a/git.c
> +++ b/git.c
> @@ -173,15 +173,21 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  				exit(0);
>  			}
>  		} else if (!strcmp(cmd, "--html-path")) {
> -			puts(system_path(GIT_HTML_PATH));
> +			char *path = system_path(GIT_HTML_PATH);
> +			puts(path);
> +			free(path);
>  			trace2_cmd_name("_query_");
>  			exit(0);
>  		} else if (!strcmp(cmd, "--man-path")) {
> -			puts(system_path(GIT_MAN_PATH));
> +			char *path = system_path(GIT_MAN_PATH);
> +			puts(path);
> +			free(path);
>  			trace2_cmd_name("_query_");
>  			exit(0);
>  		} else if (!strcmp(cmd, "--info-path")) {
> -			puts(system_path(GIT_INFO_PATH));
> +			char *path = system_path(GIT_INFO_PATH);
> +			puts(path);
> +			free(path);
>  			trace2_cmd_name("_query_");
>  			exit(0);
>  		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {

Oh interesting. These don't immediately stand out as leaky due to the
absence of intermediate variables, but nevertheless an allocation took
place that we need to free.
diff mbox series

Patch

diff --git a/git.c b/git.c
index e35af9b0e5..5eab88b472 100644
--- a/git.c
+++ b/git.c
@@ -173,15 +173,21 @@  static int handle_options(const char ***argv, int *argc, int *envchanged)
 				exit(0);
 			}
 		} else if (!strcmp(cmd, "--html-path")) {
-			puts(system_path(GIT_HTML_PATH));
+			char *path = system_path(GIT_HTML_PATH);
+			puts(path);
+			free(path);
 			trace2_cmd_name("_query_");
 			exit(0);
 		} else if (!strcmp(cmd, "--man-path")) {
-			puts(system_path(GIT_MAN_PATH));
+			char *path = system_path(GIT_MAN_PATH);
+			puts(path);
+			free(path);
 			trace2_cmd_name("_query_");
 			exit(0);
 		} else if (!strcmp(cmd, "--info-path")) {
-			puts(system_path(GIT_INFO_PATH));
+			char *path = system_path(GIT_INFO_PATH);
+			puts(path);
+			free(path);
 			trace2_cmd_name("_query_");
 			exit(0);
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {