LibreOffice Project's Check
viva64.com
viva64.com
When checking open-source projects, we almost always report the results to their authors. If we publish such a report as an article, we try to send the link to it to the developers. If there are too few bugs in a project to write an article about, we still report whatever results we've got to the authors. Or rather, we try to - sometimes developers don't (strange as it may sound) have any contacts, or their bug trackers don't accept messages, or you need to enter a captcha which no one can solve.
That's why we never send patches. There are a few reasons for that:
1. We are not familiar with the code and therefore cannot be sure if all the bugs we catch are really bugs. To understand that, we would need to study the project very closely.
2. Even with obvious bugs, we often can't say for sure how to fix them.
3. Finally, we pursue but one goal with our articles - to demonstrate the capabilities of the analyzer we develop. That is, we want to prove that our tool can find bugs in a real-life, living code. We don't aim at fixing bugs - we aim at proving that our tool can find them.
void SimpleReferenceObject::operator delete[](void * /* pPtr */)
{
free(NULL);
}
I also wonder how far PVS-Studio goes in deciding that 'expression is always true' in this fragment:
if( (pSymDef->GetType() != SbxEMPTY) ||
(pSymDef->GetType() != SbxNULL) )
class SbiSymDef {
....
SbxDataType GetType() const { return eType; }
....
};I think even MSVC2008 should accept free(NULL) without crashing.
My guess is that this was a case of tweaking the source code until the buggy compiler accepted it (it's within a "#ifdef _MSC_VER" block, so it's a MSVC-only workaround). He probably originally wrote the function containing something like std::abort(), but the compiler rejected it. So he changed it to a dummy (do-nothing) call to free(), and missed updating the comment he had written in the previous attempt.
bool getColor() {
static a = 0;
static b = 0;
a++; b++;
a %= 3;
b %= 2;
return (a && b);
}
T getColor() const;
T getColor() {
return rand(); // gotcha!
}However, the function as you wrote, when used in the expression (getColor() == getColor()), results in undefined behavior. This is because equality operator (==) is not a sequence point in C++, and thus by calling getColor() twice, you are modifying variables a & b more than once between a pair of sequence points (see "Undefined behavior" section here: http://en.cppreference.com/w/cpp/language/eval_order).
In general, you cannot be sure of the order of evaluation for a statement of form say, f() == g(). The relative order in which f & g are called is unspecified. Also, the behavior can easily become undefined if you violate certain conditions in f & g (as in the case above).
In C++14 (and I believe C++11), this is not correct. Specifically, §1.9/15 says, "Every evaluation in the calling function (including other function calls) that is not otherwise specifically sequenced before or after the execution of the body of the called function is indeterminately sequenced with respect to the execution of the called function;" this sentence is marked with a note that reads, "In other words, function executions do not interleave with each other."
Item 4 under the "Sequence point rules" section on the page you linked says basically the same thing. I haven't looked at the C++98 standard, but I imagine it allows this, too.
The two calls to "getColor" are indeterminately sequenced. As a result, the behaviour of evaluating "(getColor() == getColor())" is merely unspecified, not undefined. In fact, the behaviour is guaranteed to be equivalent to one of the possible orderings (§1.9/13: "Evaluations A and B are indeterminately sequenced when either A is sequenced before B or B is sequenced before A, but it is unspecified which").
I added a StackOverflow question (http://stackoverflow.com/questions/28816936/is-the-value-of-...) to increase the chances of someone being able to find (hopefully authoritative) information when searching for it.