Checking if pointer args are null / Field should be initialized in the member initialization list for a raw pointer

Question

I am reading about implementing Rule of Three/Five, and found examples on cppreference as well as this canonical answer on SO. I know that it doesn't make sense to use a char* in modern C++, but the point is to create some code which will need those three/five methods.

Cppreference starts with something like:

class rule_of_three
{
    char* cstring; 
    rule_of_three(const char* s, std::size_t n) // n is passed from the public ctor
        : cstring(new char[n]) // allocate
    {
        std::memcpy(cstring, s, n); // populate
    }

public:
    rule_of_three(const char* s = "")
        : rule_of_three(s, std::strlen(s) + 1) 
    { }

...

But the class doesn't check if s is null at all -- isn't this a potential problem?

I tried writing it like this:

class rule_of_three
{
    char* cstring; 
    rule_of_three(const char* s, std::size_t n) 
    {
        if (!s) throw std::runtime_error("s cannot be null");

        cstring = new char[n]; // allocate
        std::memcpy(cstring, s, n); // populate
    }

but now gcc complains that

rule_of_three::cstring should be initialized in the member initialization list [-Weffc++]

So, what would be the correct approach here?

I'm having trouble parsing what your question is. The second snippet produces a warning, don't do that then. Constructor not checking for a null argument, well, neither does strlen... (list of most of the library functions)... seems like a documentation problem. What is the sensible thing for this to do when called with a null argument? Crash is a valid option. — , Feb 07 '21 at 18:45
@dratenik: sorry, I made a mistake copying, I added a null check. — Lou, Feb 07 '21 at 20:15
Your null check is in the private constructor. When does it do something? You have to survive calling `strlen` on null in the public one first (that should normally crash). Nope, I'm still not getting it. — , Feb 07 '21 at 21:07
Turning off that warning is also a reasonable solution to this problem — M.M, Feb 07 '21 at 23:41

eerorika · Accepted Answer · 2021-02-07T22:27:15.890

But the class doesn't check if s is null at all -- isn't this a potential problem?

That is a problem only if null is passed to the constructor. Otherwise it is not.

the second snippet actually has a null check which prevents me from using the member init list.

Having a null check doesn't prevent you from using the member init list. Here is an example:

static char*
init(const char* s, std::size_t n))
{
    if (s)
        return new char[n];
    else
        throw std::runtime_error("s cannot be null");
}

rule_of_three(const char* s, std::size_t n)
    : cstring(init(s, n))
{

So, what would be the correct approach here?

Ignoring the detail that using a bare owning pointer probably isn't a correct approach in the first place: Both checking for null and not checking for null are correct approaches. One is safer and the other is more efficient. You must choose which one is more important for your use case. Regardless, initialising members is recommended.

P.S. As dratenik points out in a comment, the public constructor relies on the pointer not being null before delegating to the private constructor, so the check in the private constructor is too late.

Sorry, I made a mistake while copying code to my answer, the second snippet actually has a null check which prevents me from using the member init list. — Lou, Feb 07 '21 at 20:16

Checking if pointer args are null / Field should be initialized in the member initialization list for a raw pointer

1 Answers1