Re: Patches for issues found by automated check tools

Hi Simo,

Thanks again for the patches. I've applied most of them (see bellow) or
just fixed problems in other ways. The code is pushed to Gnome git and
I wonder if it would be possible to re-run the tools to make sure no
new issues are found? Thanks in advance!


* 0001-Check-returned-value.patch

Looks good, commited

* 0002-Fix-dead-code.patch

I don't believe the first part of the patch is correct: assert will
fire if condition is "false" which is what the original code does:

xmlSecAssert2("..." == NULL, 1)

Since string != NULL, it will fire (ugly, I know). What it actually
complains about is the return(-1) *after* that - assert always fires
and returns so second return is never reached. So to fix this one, I've
replaced assert with a real error.

The second part - Coverity is correct with the useVisa3DHack flag but
I feel much safer having the dead code that tries to free buffer instead
of not having it.

* 0003-Fix-potential-NULL-pointer-dereference.patch

Looks good, applied with a minor tweak.

* 0004-Fix-printf-format-warnings.patch

I hate printf() integer formats. Applied.

* 0005-Silence-warnings-about-unused-computed-values.patch

Don't think it matters much. Applied.

* 0006-Elimanate-assignments-that-are-never-used.patch

The keysdata.c patch is wrong - this is actually a good catch and ret
should be checked.

The pkikeys.c patch is also wrong - a check is needed.

The nss/x509vfy.c patch is badly wrong - it breaks the parser. I've set
the initial value instead.

The src/openssl/x509vfy.c is partially correct. The flag variable is
important and is actually used (I guess Clang got confused by all the
#if's in this file).

The parser.c patch is wrong too - again good catch and ret should be
checked.

Anyway, the errors were good - I just patched them in different ways.

* 0007-Fix-potential-NULL-dereference.patch

Looks good, applied with a minor tweak.



Aleksey

On 6/3/14, 2:21 PM, Simo Sorce wrote:
> Hello,
> we've run a bunch of checkers against the latest xmlsec code and I have
> a few patches you may want to consider for inclusion.
> 
> Some are no brainers, others you may like or not.
> I would like to apply them downstream once they are accepted in master,
> let me know what you think.
> 
> Simo.
> 
> 
> 
> _______________________________________________
> xmlsec mailing list
> [email protected]
> http://www.aleksey.com/mailman/listinfo/xmlsec
> 
_______________________________________________
xmlsec mailing list
[email protected]
http://www.aleksey.com/mailman/listinfo/xmlsec