crypt32: check for NULL target pointers (Coverity)

Marcus Meissner marcus at jet.franken.de
Thu Jan 7 16:16:57 CST 2010


On Thu, Jan 07, 2010 at 11:28:37AM -0800, Juan Lang wrote:
> Hi Marcus,
> 
> Ideally this patch would be accompanied by a test.  Also, this change:
> 
> @@ -212,6 +212,10 @@ static BOOL CRYPT_DecodeEnsureSpace(DWORD dwFlags,
> 
>      if (dwFlags & CRYPT_DECODE_ALLOC_FLAG)
>      {
> +        if (!pvStructInfo) {
> +            SetLastError(ERROR_INVALID_PARAMETER);
> +            return FALSE;
> +        }
> 
> is a noop in all but one case:  all the callers of
> CRYPT_DecodeEnsureSpace, save one, check pvStructInfo before calling
> it.  It would be clearer, IMO, to change the single caller that
> doesn't check pvStructInfo (CryptDecodeObjectEx) rather than adding a
> check that is useless in most cases.  A similar statement applies to
> the encode.c change:  just change CryptEncodeObjectEx, not
> CRYPT_EncodeEnsureSpace.
> 
> Finally, please indent consistently with the rest of the file.
> 
> If you prefer, I can try to fix this.  Triaging the Coverity bugs is
> probably enough work by itself, without being expected to fix them too
> ;-)  Thanks,

In the meantime I had marked them as IGNORE already, as Windows also
shows inconsistent behaviour...

I would try this patch, although I am not sure I matched your indent style,
it is a bit strange:




More information about the wine-devel mailing list