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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 17, 2018, 09:28 UTC
Message-ID: <CAPig+cQVWY3+2aarYw=uXti0=1SW8boMPoYj1zatw1KKKVOqnQ@mail.gmail.com>
In-Reply-To: <20180817091710.1767-1-mgorny@gentoo.org>

On Fri, Aug 17, 2018 at 5:17 AM Michał Górny <mgorny@gentoo.org> wrote:

Show 5 quoted lines

> 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?

[1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html

Show 25 quoted lines

> Signed-off-by: Michał Górny <mgorny@gentoo.org>
> ---
> 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);
>  }