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.

It seems you are invoking string constructor with null pointer which is not allowed. — dewaffled, Jun 25 '20 at 08:32
Does this answer your question? [Assign a nullptr to a std::string is safe?](https://stackoverflow.com/questions/10771864/assign-a-nullptr-to-a-stdstring-is-safe) — dewaffled, Jun 25 '20 at 08:33
`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. — Thomas Sablik, Jun 25 '20 at 08:33
You're not allowed to modify the result of `input.c_str()`. Use `std::string` functions and `` instead of the old C interface. — molbdnilo, Jun 25 '20 at 08:35
The short and safe version: `std::istringstream stream(input); std::copy(std::istream_iterator(stream), std::istream_iterator(), std::back_inserter(v));`. — molbdnilo, Jun 25 '20 at 08:40

score 7 · Accepted Answer · answered Jun 25 '20 at 08:39

7

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";
    }
}

answered Jun 25 '20 at 08:39

Alan Birtles

32,622
4
31
60

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? – Aman Kumar Jun 25 '20 at 11:58
@ProfessorofStupidity The vector `v` is the same object, but the loop variable `s` works the same as other variables. – molbdnilo Jun 25 '20 at 12:44
@ProfessorofStupidity without the reference you would be making a copy of the strings in the vector which isn't necessary – Alan Birtles Jun 25 '20 at 14:04

Yksisarvinen · Answer 2 · 2020-06-25T08:45:44.523

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.

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. — molbdnilo, Jun 25 '20 at 08:43

score 1 · Answer 3 · answered Jun 25 '20 at 08:59

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.

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...

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;
 }

what(): basic_string::_M_construct null not valid

3 Answers3