[xmlsec] Patches for issues found by automated check tools

Aleksey Sanin aleksey at aleksey.com
Wed Jun 4 19:46:25 PDT 2014


> Not that I care much, but why did you change the commit author ?
> Other projects leave credit in git commits ...

I had some problems with 'git apply' in the past so I am using the
old trusted 'patch' :) I noted that these patches came from you
in the comments, for example:

commit b4baa10befc8bcdd8a7f91daa68a56ff3940546d
Author: Aleksey Sanin <aleksey at aleksey.com>
Date:   Wed Jun 4 09:53:02 2014 -0700

    unused variables - Clang complains (Simo Sorce, Red Hat)


>> 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.
> 
> Maybe just set a return error variable and "goto done;" ?
> 
> That way further modification won't risk forgetting and you avoid
> unnecessary duplication of boilerplate.

Agree. I just don't think that 6 lines of 'dead' code are worth
refactoring right now :)

> 
>> The nss/x509vfy.c patch is badly wrong - it breaks the parser. I've set
>> the initial value instead.
> 
> Not sure why you say this, the error was not missing initialization, but
> that valueLen set in the else branch is never used.
> Looking at the code valueLen all the time you set valueLen before using
> it. It is never used untouched after you set it in the else branch,
> hence why the checker correclty marked it as dead code. My manual
> inspection seem to confirm it, but maybe there is some macro that uses
> it I am not seeing ?
> Initializing it is also useless because you never use that value unless
> you first initialized it with the return of
> xmlSecNssX509NAmeStringRead() in one of the 2 if branches.
> 

You are right. Got confused with the big while() loop.

> I was able to make only a coverity run on master and all errors for
> which I sent patches are gone (I have a couple false positives I didn't
> bother with), except the dead_code warnings about free(buf) as expected.
> 
> I will try to do the other scans in the next few days.
> 

Thanks a lot!

Aleksey




More information about the xmlsec mailing list