Cocos2d-x checked with PVS-Studio

PVS-Studio is a cool code analysis tool developed by a russian team. Recently they checked cocos2d-x and wrote an article with detailed results: http://www.viva64.com/en/b/0275/

Conclusion
Like I’ve already told you in the very beginning, there are pretty few suspicious fragments in the Cocos2d-x project. This framework is relatively young and innovative and doesn’t contain any legacy code from the old times. The project developers seem to be using various means of code quality control and trying to conform to modern standards and programming methodologies.

Nice, but I have one point, I am not agreeing with:

If the ‘new’ operator has failed to allocate memory, the std::bad_alloc() exception will be generated, according to the C++ language standard. So, checking the pointer for being null doesn’t make sense, unlike the return value of the ‘malloc’ function.

This will only work, if cocos2d-x would use exception handling, which is not the case.

Cocos2d-x doesn’t use exception handling indeed, but he has a point, there should be a (std::nothrow) there for the “new” to return a nullptr in case of failure.

Emmmm… what?
Operator new will return nullptr in case of failure only if you call it with std::nothrow parameter, like as follows

auto something = new(std::nothrow) SomeClass;
if (!something)
{
   std::cerr  << "Oh God! 'new' failed. We all gonna die!!" << std::endl;
}

Otherwise it should throw std::bad_alloc.
Read the standard, please. https://isocpp.org/files/papers/N3690.pdf § 3.7.4.1, page 64.

Yes. I was just referring to the sentence:

So, checking the pointer for being null doesn’t make sense, unlike the return value of the ‘malloc’ function.

I does not make sense, if not using the nothrow allocation, but maybe that was what they wanted to point out.
And checking the pointer for null, by using the nothrow allocation, is just checking the return value of the malloc function.

Using std::nothrow is NOT exception handling. It’s a nothrow allocation.
Exception handling is by executing code in the handler’s try block.

Read the standard, please. https://isocpp.org/files/papers/N3690.pdf § 15, page 386.

I can’t understand what are you trying to prove?
If you does not use try-catch block around new (or any other code, which may throw), it does not mean that new won’t throw an exception. It means that if it throws an exception, you’ll get an uncaught exception which eventually leads to std::terminate()

Look, the OP was posting this:

If the ‘new’ operator has failed to allocate memory, the std::bad_alloc() exception will be generated, according to the C++ language standard.

You can only react to a std::bad_alloc() exception, if you are using exception handling(try/catch).

The OP also said:

So, checking the pointer for being null doesn’t make sense, unlike the return value of the ‘malloc’ function.

I find fault with the word UNLIKE, because if you are using std::nothrow, checking the pointer being null is exactly checking the return value of the malloc function!

On the one side the post says check the return value(std::bad_alloc()) of std::malloc(which is only possible with EH), but on the other side he concurrently says “checking the pointer for being null doesn’t make sense”.
It makes perfect sense, when using std::nothrow.

The wording of the OP is just wrong, cause it is not considering the usage of std::nothrow, but only the usage of exception handling.

No. You don’t get an uncaught exception, as no handlers at all are getting called.
It will only throw an exception, if your code was compiled with exception handling support.

You can only catch exceptions in try/catch blocks and even throw exceptions from try blocks or from functions called inside the try block. If you are not using try/catch blocks, no handler at all will be called, not even the terminate handler.

std::terminate() is called by the C++ runtime when exception handling fails

If you don’t use exception handling(try/catch), you are not catching ANY exception as the handlers are not called. std::terminate() is a handler itself and so it’s also not called.

From the standard:

A handler will be invoked only by throwing an exception in code executed in the handler’s try block or in functions called from the handler’s try block.

I’m not connected in any way with PVS-Studio dev team btw )
Let’s just make an experiment

#include <iostream>

const long long huge = 10000000;

class A {
    int a[huge][huge];
};

int main(int argc, const char* argv[]) {
    A* a1 = new(std::nothrow) A;
    if (a1 == NULL) {
        std::cout << "NULL POINTER" << std::endl;
    }
    
    try {
        A* a2 = new A;
    } catch (std::bad_alloc& ba) {
        std::cout << "EXCEPTION" << std::endl;
    }
    
    A* a3 = new A;
    std::cout << "WON'T MAKE IT HERE HAHAHA" << std::endl;
}

Both clang and gcc give you the same results:

NULL POINTER
EXCEPTION
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc

Termination mechanism may be system dependent, but we may conclude that checking a pointer against nullptr makes no sense if you just call new Someclass(). If you use new(std::nothrow) Someclass() it does.

correct.
we should use new (std::nothrow) SomeClass.
We started migrating to that, but we haven’t finished it yet.

On which OS have you tried it?

The compiler would check for stack size at compile time.

E.g. compiling this code on Windows with mingw(g++ (rev5, Built by MinGW-W64 project) 4.8.1) will give you a compiler error anyway:

test.cpp:6:21: error: size of array 'a' is too large
     int a[huge][huge];

OS X and whatever is used here: http://www.compileonline.com/compile_cpp11_online.php

Does Cocos2d-x use std::nothrow? No, it does not. The article describes a specific code - the code of Cocos2d-x, not some abstract hypothetical code.

Try this one:

#include <iostream>

struct Type
{
    char internal[1024];
};

const size_t alot = 1u << 31;

int main()
{
	Type* t = new Type[alot];

	// compiler may optimize out the allocation if there are no side effects
	std::cout << "Ok: " << *t[0].internal << std::endl;

	delete[] t;
	return 0;
}