How to best protect from 0 passed to std::string parameters?

Question

I have just realized something disturbing. Every time I have written a method that accepts a std::string as a paramater, I have opened up myself to undefined behaviour.

For example, this...

void myMethod(const std::string& s) { 
    /* Do something with s. */
}

...can be called like this...

char* s = 0;
myMethod(s);

...and there's nothing I can do to prevent it (that I am aware of).

So my question is: How does someone defend themself from this?

The only approach that comes to mind is to always write two versions of any method that accepts an std::string as a parameter, like this:

void myMethod(const std::string& s) {
    /* Do something. */
}

void myMethod(char* s) {
    if (s == 0) {
        throw std::exception("Null passed.");
    } else {
        myMethod(string(s));
    }
}

Is this a common and/or acceptable solution?

EDIT: Some have pointed out that I should accept const std::string& s instead of std::string s as a parameter. I agree. I modified the post. I don't think that changes the answer though.

Answer 1

I don't think you should protect yourself. It's undefined behavior on the caller's side. It's not you, it's the caller who calls std::string::string(nullptr), which is what is not allowed. The compiler lets it be compiled, but it lets other undefined behaviors be compiled as well.

The same way would be getting "null reference":

int* p = nullptr;
f(*p);
void f(int& x) { x = 0; /* bang! */ }

The one who dereferences the null pointer is making UB and is responsible for it.

Moreover, you cannot protect yourself after undefined behavior has happened, because the optimizer is in its full right to assume that the undefined behavior had never happened, so the checks if c_str() is null can be optimized out.

Answer 2

I also encountered this problem a few years ago and I found it to be a very scary thing indeed. It can happen by passing a nullptr or by accidentally passing an int with value 0. It's really absurd:

std::string s(1); // compile error
std::string s(0); // runtime error

However, in the end this only bugged me a couple of times. And each time it caused an immediate crash when testing my code. So no night-long sessions will be required to fix it.

I think overloading the function with const char* is a good idea.

void foo(std::string s)
{
    // work
}

void foo(const char* s) // use const char* rather than char* (binds to string literals)
{
    assert(s); // I would use assert or std::abort in case of nullptr . 
    foo(std::string(s));
}

I wish a nicer solution was possible. However, there isn't.

Answer 3

The code below gives a compile failure for an explicit pass of 0, and a run-time failure for a char* with value 0.

Note that I do not imply that one should normally do this, but no doubt there may be cases where protection from the caller error is justified.

struct Test
{
    template<class T> void myMethod(T s);
};

template<> inline void Test::myMethod(const std::string& s)
{
    std::cout << "Cool " << std::endl;
}

template<> inline void Test::myMethod(const char* s)
{
    if (s != 0)
        myMethod(std::string(s));
    else
    {
        throw "Bad bad bad";
    }
}

template<class T> inline void Test::myMethod(T  s)
{
    myMethod(std::string(s));
    const bool ok = !std::is_same<T,int>::value;
    static_assert(ok, "oops");
}

int main()
{
    Test t;
    std::string s ("a");
    t.myMethod("b");
    const char* c = "c";
    t.myMethod(c);
    const char* d = 0;
    t.myMethod(d); // run time exception
    t.myMethod(0); // Compile failure
}

Answer 4

How about changing the signature of your method to:

void myMethod(std::string& s) // maybe throw const in there too.

That way the caller has to create a string before they call it, and the sloppiness you are worried about will cause problems before it gets to your method, making obvious, what others have pointed out, that it is the caller's error not yours.

Answer 5

How about you provide on overload the takes an int parameter?

public:
    void myMethod(const std::string& s)
    { 
        /* Do something with s. */
    }    

private:
    void myMethod(int);

You don't even have to define the overload. Trying to call myMethod(0) will trigger a linker error.

Answer 6

Your method in the first code block will never be called if you try to call it with a (char *)0. C++ will simply try to create a string and it will throw the exception for you. Did you try it?

#include <cstdlib>
#include <iostream>

void myMethod(std::string s) {
    std::cout << "s=" << s << "\n";
}

int main(int argc,char **argv) {
    char *s = 0;
    myMethod(s);
    return(0);
}


$ g++ -g -o x x.cpp 
$ lldb x 
(lldb) run
Process 2137 launched: '/Users/simsong/x' (x86_64)
Process 2137 stopped
* thread #1: tid = 0x49b8, 0x00007fff99bf9812 libsystem_c.dylib`strlen + 18, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00007fff99bf9812 libsystem_c.dylib`strlen + 18
libsystem_c.dylib`strlen + 18:
-> 0x7fff99bf9812:  pcmpeqb (%rdi), %xmm0
   0x7fff99bf9816:  pmovmskb %xmm0, %esi
   0x7fff99bf981a:  andq   $15, %rcx
   0x7fff99bf981e:  orq    $-1, %rax
(lldb) bt
* thread #1: tid = 0x49b8, 0x00007fff99bf9812 libsystem_c.dylib`strlen + 18, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00007fff99bf9812 libsystem_c.dylib`strlen + 18
    frame #1: 0x000000010000077a x`main [inlined] std::__1::char_traits<char>::length(__s=0x0000000000000000) + 122 at string:644
    frame #2: 0x000000010000075d x`main [inlined] std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(this=0x00007fff5fbff548, __s=0x0000000000000000) + 8 at string:1856
    frame #3: 0x0000000100000755 x`main [inlined] std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(this=0x00007fff5fbff548, __s=0x0000000000000000) at string:1857
    frame #4: 0x0000000100000755 x`main(argc=1, argv=0x00007fff5fbff5e0) + 85 at x.cpp:10
    frame #5: 0x00007fff92ea25fd libdyld.dylib`start + 1
(lldb) 

See? You have nothing to worry about.

Now if you want to catch this a bit more gracefully, you should simply not be using char *, then the problem won't arise.

Answer 7

If you are worried about char* being potentially null pointers (e.g. returns from external C APIs) the answer is to use a const char* version of the function instead of the std::string one. i.e.

void myMethod(const char* c) { 
    std::string s(c ? c : "");
    /* Do something with s. */
}

Of course you will need the std::string version as well if you want to allow those to be used.

In general it's best to isolate calls to external APIs and marshal arguments into valid values.