Re: [PATCH 02/29] parse-options: fix leaks for users of OPT_FILENAME
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jun 6, 2024, 10:00 UTC
- Message-ID
- <CAOLa=ZQj1UrNO+nOxhp2kaO+F1hHDE14FMYi74Eg8sMAgXz8OQ@mail.gmail.com>
- In-Reply-To
- <ecabbb74e19ea773fd23719bd7fd4937b29679e5.1717402439.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 22 quoted lines
> diff --git a/builtin/archive.c b/builtin/archive.c
> index 15ee1ec7bb..f29c0ef6ad 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -92,6 +92,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
> N_("path to the remote git-upload-archive command")),
> OPT_END()
> };
> + int ret;
>
> argc = parse_options(argc, argv, prefix, local_opts, NULL,
> PARSE_OPT_KEEP_ALL);
> @@ -106,6 +107,8 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
>
> setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
>
> - UNLEAK(output);
> - return write_archive(argc, argv, prefix, the_repository, output, 0);
> + ret = write_archive(argc, argv, prefix, the_repository, output, 0);
> +
> + free(output);
> + return ret;Since output is passed to `write_archive`, we capture the return and free output. Makes sense.
[snip]
Show 23 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 78a247d8a9..4e4b645a21 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2021,7 +2021,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> const char *rfc = NULL;
> int creation_factor = -1;
> const char *signature = git_version_string;
> - const char *signature_file_arg = NULL;
> + char *signature_file_arg = NULL;
> struct keep_callback_data keep_callback_data = {
> .cfg = &cfg,
> .revs = &rev,
> @@ -2559,6 +2559,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> strbuf_release(&rdiff1);
> strbuf_release(&rdiff2);
> strbuf_release(&rdiff_title);
> + free(description_file);
> + free(signature_file_arg);
> free(to_free);
> free(rev.message_id);
> if (rev.ref_message_ids)
>I see `char *description_file = NULL` already and it is also used `OPT_FILENAME`. So all good here.
The rest of the patch looks good too. This was commit was quite easy to ready. Appreciate the neat breakdown.