Fix code errors identified by PVS-Studio check

Thanks Egor,

If you're able to attach a copy of the full analysis report, that would be much appreciated.

I have fixed a number of the bugs (in trunk) already:

* Dangerous parameter redefinition - fixed
* Incorrectly commented-out line - fixed
* Lost comma - fixed
* Wrong length in strncomp - fixed
* Missing semicolon - fixed
* Incorrect object name in a condition - fixed
* Careless use of numeric constants - fixed

* Testing a pointer for null after new.

  Are we allowed to use smart pointers? (It's not listed as allowed in the C++11 features wiki page.) And if so, how does one do it?

  For a quick fix, it looks like one can insert (std::nothrow):

  srcBuf = new (std::nothrow) Byte [srcLen];

Smart pointers should be fine to use (it was mostly implemented in TR1 anyway). Basically, you just replace:

Thing* my_thing = new Thing();
// do stuff
delete my_thing;

with:

unique_ptr<Thing> my_thing(new Thing());

You can also get rid of the "delete" command, as the Thing will automatically be deleted when it goes out of scope.

If the pointer will be shared by multiple containers, you need to use shared_ptr instead of unique_ptr.

Useful comments from the developer list:

Mathog:
> There are instances where coding logic as a one time loop is convenient
> because then "break" may be used instead of "goto". In some programming
> projects a religious dispute results if a goto is employed, and this
> avoids that conflict.
> As in:
>
> // not really a loop
> while(1){
> if(condition1)break;
> //code
> if(condition2)break;
> //code
> //etc.
> break;
> }
>
> The object file coming out of the compiler probably differs not at all
> from the goto variant, since the unconditional entry and the final
> unconditional break tells it that this isn't really a repeating section
> of code.

Here, the analyser says: ‘I'm not sure what the author really wanted this code to do.’ and is just not very clever as it's quite clear in the code.

Michael Soegtrop:
> I would be careful with the this!=NULL statement for
>
> bool SPLPEItem::performPathEffect
>
> as far as I can tell, this is not a virtual function, and there is no
> reason why the this pointer of a non virtual function should not be
> NULL. One can call a non virtual member function on a NULL pointer. Not
> very nice, but some people put such checks into the code as extra safety
> check on purpose and some even use tricks like calling a function on
> NULL pointers on purpose, e.g. for saving a lot of checks when the
> function is called.

Therefore the comment ‘this expression is always false on newer compilers, because 'this' pointer can never be NULL’ by the analyser is probably wrong.

* Identical Expressions:

I believe the following is the correct fix, but perhaps someone more familiar with lib2geom should check:

if(depth > 12 || (Ar.maxExtent() < tol && Ar.maxExtent() < tol)) {

                                          |
                                          V

if(depth > 12 || (Ar.maxExtent() < tol && Br.maxExtent() < tol)) {

* Very odd method:

I disagree with the analyzer's solution; it should be:

if (back != nVal) {
    back = nVal;
    ResetPoints();
}

with the surrounding if statement.