Re: Patches for issues found by automated check tools

Aleksey Sanin <[email protected]>
Newsgroups gmane.text.xml.xmlsec
Message-ID <[email protected]>
> 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 <[email protected]>
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


_______________________________________________
xmlsec mailing list
[email protected]
http://www.aleksey.com/mailman/listinfo/xmlsec
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.