Re: [sr #107539] Patch: crl.c: pointer targets differ in signedness

[Jeffrey Walton]

On Thu, Nov 25, 2010 at 1:00 PM, Simon Josefsson
<address@hidden> wrote:
>
> Follow-up Comment #1, sr #107539 (project gnutls):
>
> Re this code:
>
> +  if (crl == NULL)
> +    {
> +      gnutls_assert ();
> +      /* Need a GNUTLS_E_INVALID_PARAMETER */
> +      return GNUTLS_E_SHORT_MEMORY_BUFFER;
> +    }
>
> The error typically used is GNUTLS_E_INVALID_REQUEST.
OK. Got it.

>
> +      opaque *out = NULL;
>
> What's the reason for this?  It is initialized later on.
Under the debugger, its hard to tell what is valid (appears to be
garbage) and what is uninitialized (is really garbage). If the
initialization is not needed, the optimizer will drop it.

> +  if (buf == NULL || sizeof_buf == NULL)
> +    {
> +      gnutls_assert ();
> +      return GNUTLS_E_INVALID_REQUEST;
> +    }
>
> This is quite wrong: read the documentation for the function, buf can be
> NULL.  The same applies to a couple of more instance, and I stopped reading.
OK. I caught that after submission :/

> Some of the stuff is good, so please rework the patch and I'll review again.
OK. Thanks.

Jeff