
LibreOffice Project's Check - mmastrac
http://www.viva64.com/en/b/0308/#ID0ECRAI
======
KNoureen
Are these issues reported to LO devs, or they have to look it up themselves?

~~~
AndreyKarpov
Readers' FAQ on Articles about PVS-Studio and CppCat:
[http://www.viva64.com/en/a/0085/](http://www.viva64.com/en/a/0085/)

~~~
mintplant
_Have you reported the bugs to the project developers? Have you sent them a
patch?

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._

------
cremno
>So do implement it then to avoid the compilation and/or linking errors, but
make it crash intentionally if called.

    
    
        void SimpleReferenceObject::operator delete[](void * /* pPtr */)
        {
          free(NULL);
        }
    

What will cause the crash? Passing a null pointer to free() is fine. Something
specific to C++?

~~~
EpicEng
Per the spec, free(NULL) is fine and well defined, essentially a no-op. Not
sure what they're thinking. abort would have been an option.... And hell, were
they trying to invoke UB and assuming that would cause a crash? Wrong on
multiple levels.

~~~
pgeorgi
Spec is fine and all, but free(NULL) reliably crashes on Solaris and probably
other systems.

~~~
cremno
The malloc(3C) man pages since 5.5.1 claim otherwise.

------
twotwotwo
These are especially interesting to me for the rules--it is not crazy hard to
add things to Go's `go vet` tool (or write other checks outside vet with
`go/ast`, etc.), and I wonder if "assigned variable twice consecutively" or
"foo() == foo() looks suspicious" are good candidates.

------
GFK_of_xmaspast
Could the first example, with "getColor() == getColor()", in general, be a
false positive if getColor() has side effects?

    
    
      bool getColor() {
       static a = 0;
       static b = 0;
       a++; b++;
       a %= 3;
       b %= 2;
       return (a && b);
      }
    

or does something in the standard prevent this?

~~~
anuragbiyani
It's perfectly valid for the function getColor() to return different value for
each invocation, e.g., think std::rand().

 _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](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).

~~~
sjolsen
>the function as you wrote, when used in the expression (getColor() ==
getColor()), results in undefined behavior

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").

~~~
anuragbiyani
Thanks for correcting me _sjolsen_. You are right, the behavior is unspecified
only.

I added a StackOverflow question
([http://stackoverflow.com/questions/28816936/is-the-value-
of-...](http://stackoverflow.com/questions/28816936/is-the-value-of-
expression-f-g-when-f-g-modify-same-global-variable-und)) to increase the
chances of someone being able to find (hopefully authoritative) information
when searching for it.

