what(): basic_string::_M_construct null not valid

Question

I am making a program where I need to use a function which stores a tokens of a string in a vector. The function did not work properly so I tried the function on a smaller program. Of course, I used string tokenizer function. But it is not working as expected. First, here's the code:

#include <vector>
#include <string>
#include <cstring>
using namespace std;

int main()
{
    vector<string> v;
    string input = "My name is Aman Kumar";
    
    char* ptr = strtok((char*)input.c_str(), " ");
    v.push_back((string)ptr);

    while(ptr)
    {
        ptr = strtok(NULL," ");
        v.push_back((string)ptr);
    }
    cout<<"Coming out";
    for(string s:v)
    {
        cout<<s<<endl;
    }
}

Now the problems. I think the issue has something to do with the command:

(string)ptr

This thing works perfectly in the first call, but gives error when present in the while loop. If I comment it out and print ptr, then it works okay, but then the program terminates after the while loop, and doesn't even execute

cout<<"coming out";

leave alone the contents of the vector. But again, if I don't print ptr too, then the first Token "My" which was stored in the vector gets printed. I literally cannot find what is causing this. Any suggestion would be helpful.

Desert

It seems you are invoking string constructor with null pointer which is not allowed. – Fortaleza

Does this answer your question? Assign a nullptr to a std::string is safe? – Fortaleza

char* ptr = strtok((char*)input.c_str(), " "); this could be a problem. c_str() returns const C string and you must not change it. But you are removing the constness with (char*)input.c_str() and passing it to strtok. strtok modifies the strings. That causes undefined behavior. – Briticism

You're not allowed to modify the result of input.c_str(). Use std::string functions and <algorithm> instead of the old C interface. – Naresh

The short and safe version: std::istringstream stream(input); std::copy(std::istream_iterator<string>(stream), std::istream_iterator<string>(), std::back_inserter(v));. – Naresh

Answer 1

In

while(ptr)
{
    ptr = strtok(NULL," ");
    v.push_back((string)ptr);
}

For the last token ptr will be null, constructing a std::string from a null pointer is undefined behaviour. Try:

while(ptr = strtok(NULL," "))
{
    v.push_back(string(ptr));
}

For a more c++ solution:

#include <vector>
#include <string>
#include <iostream>
#include <sstream>

int main()
{
    std::vector<std::string> v;
    std::string input = "My name is Aman Kumar";
    
    std::stringstream ss(input);
    
    std::string word;
    while(ss >> word)
    {
        v.push_back(word);
    }
    std::cout << "Coming out\n";
    for(std::string& s:v)
    {
        std::cout << s << "\n";
    }
}

Louvar

Thanks a lot. Can you tell me why in the last for loop you have used string& instead of string? For loop surely have the same object and not a copy object, right? – Desert

@ProfessorofStupidity The vector v is the same object, but the loop variable s works the same as other variables. – Naresh

@ProfessorofStupidity without the reference you would be making a copy of the strings in the vector which isn't necessary – Louvar

Answer 2

You don't know if ptr is nullptr before you try to create string out of it (and calling std::string construtor with nullptr is UB)

You need to reorganize your loop, e.g. like this:

char* ptr = strtok(input.data(), " ");

while(ptr)
{
    v.push_back(std::string(ptr));
    ptr = strtok(NULL," ");
}

As a side note, don't use C-style cast syntax in C++. It it very likely to hide problems, and C++ syntax offers much safer alternatives.

Casting away constness and modifying result is UB in C++, so first cast can be replaced with data call (which returns pointer to non-const when needed). If you don't have C++11, then this is UB anyway, because string was not guaranteed to store memory in continuous memory and you need to use different methods.

Grodno

Casting away constness is not a problem (it would be impossible to interface with a lot of C code otherwise). It's subsequent modifications that cause undefined behaviour. – Naresh

Answer 3

You are mixing std::string with the C-ish strtok function. That is really a bad idea. std::string are more or less intervertible with const char * but not with mutable char *. Not speaking of the null pointer question. So choose one method and stick to it.

1. C-ish one: build a plain char array and store vectors of char *:

    char *cstr = strdup(input.c_str());
    ptr = strtok(cstr, " ");
    while(ptr) {
        v.push_back(ptr);
        ptr = strtok(NULL, " ");
    }
   
    cout<<"Coming out";
    for(char *s:v)
    {
        cout<<s<<endl;
    }
   
    free(cstr);         // always free what has been allocated...
   

2. C++ one, use a std::stringstream:

    std::stringstream fd(input);
    for(;;) {
        std::string ptr;
        fd >> ptr;
        if (! fd) break;
        v.push_back(ptr);
    }
   
    cout<<"Coming out";
    for(std::string s: v)
    {
        cout<<s<<endl;
    }
   

Thissa