Re: [PATCH 18/22] builtin/format-patch: fix various trivial memory leaks
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 8, 2024, 05:05 UTC
- Message-ID
- <ZrRSB46ZxHFU1X-5@tanuki>
- In-Reply-To
- <D39JV83QXD5C.3U74JE9YD5SGO@jamesliu.io>
On Wed, Aug 07, 2024 at 06:51:52PM +1000, James Liu wrote:
Show 23 quoted lines
> > diff --git a/builtin/log.c b/builtin/log.c
> > index a73a767606..ff997a0d0e 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -2023,6 +2024,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;
> > + char *signature_to_free = NULL;
> > char *signature_file_arg = NULL;
> > struct keep_callback_data keep_callback_data = {
> > .cfg = &cfg,
> > @@ -2443,7 +2445,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >
> > if (strbuf_read_file(&buf, signature_file, 128) < 0)
> > die_errno(_("unable to read signature file '%s'"), signature_file);
> > - signature = strbuf_detach(&buf, NULL);
> > + signature = signature_to_free = strbuf_detach(&buf, NULL);
>
> Do I understand this correctly, that the multiple assignment here allows
> us to maintain a reference to the pointer returned by `strbuf_detach()`
> in `signature_to_free`, and we do this because `signature` can take on a
> different value below?Not only below, but also its default value is `git_version_string`, which is a string constant. So we use the multiple assignment such that we can avoid freeing `signature`, which may contain string constants, and unconditionally free `signature_to_free` because that variable always holds an allocated string or a `NULL` pointer.
Patrick
Show 5 quoted lines
> > } else if (cfg.signature) {
> > signature = cfg.signature;
> > }
>
>