On Fri, 2018-08-17 at 05:28 -0400, Eric Sunshine wrote:
> On Fri, Aug 17, 2018 at 5:17 AM Michał Górny <mgorny@xxxxxxxxxx> wrote:
> > Fix signature_check_clear() to free only values that are non-NULL. This
> > especially applies to 'key' and 'signer' members that can be NULL during
> > normal operations, depending on exact GnuPG output. While at it, also
> > allow other members to be NULL to make the function easier to use,
> > even if there is no real need to account for that right now.
>
> free(NULL) is valid behavior[1] and much of the Git codebase relies upon it.
>
> Did you run into a case where it misbehaved?
Nope. I was actually wondering if it's expected, so I did a quick grep
to check whether git is checking pointers for non-NULL before free()ing,
and found at least one:
blame.c-static void drop_origin_blob(struct blame_origin *o)
blame.c-{
blame.c- if (o->file.ptr) {
blame.c: FREE_AND_NULL(o->file.ptr);
blame.c- }
blame.c-}
So I wrongly presumed it might be desirable. If it's not, that's fine
by me.
>
> [1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html
>
> > Signed-off-by: Michał Górny <mgorny@xxxxxxxxxx>
> > ---
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index 35c25106a..9aedaf464 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -15,9 +15,14 @@ static const char *gpg_program = "gpg";
> > void signature_check_clear(struct signature_check *sigc)
> > {
> > - FREE_AND_NULL(sigc->payload);
> > - FREE_AND_NULL(sigc->gpg_output);
> > - FREE_AND_NULL(sigc->gpg_status);
> > - FREE_AND_NULL(sigc->signer);
> > - FREE_AND_NULL(sigc->key);
> > + if (sigc->payload)
> > + FREE_AND_NULL(sigc->payload);
> > + if (sigc->gpg_output)
> > + FREE_AND_NULL(sigc->gpg_output);
> > + if (sigc->gpg_status)
> > + FREE_AND_NULL(sigc->gpg_status);
> > + if (sigc->signer)
> > + FREE_AND_NULL(sigc->signer);
> > + if (sigc->key)
> > + FREE_AND_NULL(sigc->key);
> > }
--
Best regards,
Michał Górny
Attachment:
signature.asc
Description: This is a digitally signed message part
