Re: [PATCH] gpg-interface.c: Fix potentially freeing NULL values

On Fri, 2018-08-17 at 05:28 -0400, Eric Sunshine wrote:

On Fri, Aug 17, 2018 at 5:17 AM Michał Górny [off-list ref] wrote:

quoted

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

quoted

Signed-off-by: Michał Górny <redacted>
---

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