On Wed, Aug 07, 2024 at 06:51:52PM +1000, James Liu wrote:
> > 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
> > } else if (cfg.signature) {
> > signature = cfg.signature;
> > }
>
>
Attachment:
signature.asc
Description: PGP signature
