Comment by jraph | Hacker News Reader

Comment by jraph | Hacker News Reader

Parent Full thread

> Why is a separate static analyzer necessary for this

Because this is a nail this co-founder of PVS-Studio saw while holding the hammer he wants to sell.

By the way, it seems to me this error does not have much to do with comparison functions, it is a mistake that can be made in many kinds of functions.

Unused parameters are warned against in C (hence all the UNUSED macro hacks) and no static analyzers are going to save you if you don't address compiler warnings, they are just going to add more warnings to that pile of warnings you already have (or maybe don't get because the right warnings are not enabled).

(I'm sure their blog is full of cases a static analyzer would handle which compilers won't warn you for though)

Still surprising to see such an error in OpenSSL and like others here I would be interested to know why it was not caught.

Last time OpenSSL was in the news for a bug that should generate a warning I took a cursory look at some of the code, it was stuffed with stuff that would generate similar warnings. So at that time I concluded that going through all the warnings would be a pretty big job, and that is likely why nobody did so. On the more speculative front, it might be that some maintainer is against taking whitespace patches, that would probably make anyone who tries give up pretty quickly.

I'm a bit skeptical of the claim "The code quality is excellent", if it was this would be a lone warning emitted by the compiler, and surely it would have been fixed then.

“The code quality is excellent” is either the author hedging pissing people off who are fans of OpenSSL or their analyzer doesn’t spot a lot of issues and thus they conclude it’s excellent for those reasons.

Actually the author said that the “The code quality is excellent” specifically about the new version 3.0 in contrast with the older OpenSSL versions, where he could find much more problems.

I have not looked yet at the new version, which has a very large number of changes, but it is quite possible that the author is right and that the rewritten code is very good.

I think comparison functions are a prime "use case" for this kind of error because it will pass the type check easily and comparison is often somewhat symmetric, so you will do a lot of copy and paste. Only unit or property tests could catch this (and a compiler warning, of course). The example here is of course extreme, but I bet you will find something like this quite often, especially in OO code like Java.

Reply on news.ycombinator.com