Why isn't C++ strtok() working for me?

Question

The program is supposed to receive an input through cin, tokenize it, and then output each one to show me that it worked properly. It did not.

The program compiles with no errors, and takes an input, but fails to output anything.

What am I doing wrong?

int main(int argc, char* argv[])
{
  string input_line;

  while(std::cin >> input_line){
    char* pch = (char*)malloc( sizeof( char ) *(input_line.length() +1) );

    char *p = strtok(pch, " ");
    while (p != NULL) {
      printf ("Token: %s\n", p);
      p = strtok(NULL, " ");
    }
  }
  return 0;
}

I followed the code example here: http://www.cplusplus.com/reference/clibrary/cstring/strtok/

Thanks.

Answer 1

Looks like you forget to copy the contents of input_line to pch:

strcpy(pch, input_line.c_str());

But I'm not sure why you're doing string tokenization anyway. Doing cin >> input_line will not read a line, but a token.. so you get tokens anyway?

Answer 2

You didn't initialize your string. Insert

strcpy(pch, input_line.c_str());

after the malloc line.

Answer 3

Or use this:

pch = strdup(input_line.c_str());

Hope this helps, Best regards, Tom.

Answer 4

This is more of a correctness post, Hans has your problem.

The correct way to get a line of input is with getline:

std::string s;
std::getline(std::cin, s);

std::cin breaks at whitespace anyway, so if you typed asd 123 and ran your code, input_line would first be "asd", then the second time in the loop "123" (without waiting for enter).

That said, an easy way to get your result is with a stringstream. Any time you explicitly allocate memory, especially with malloc, you're probably doing something the hard way. Here's one possible solution to tokenizing a string:

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

int main(void)
{
    std::string input;
    std::getline(std::cin, input);

    std::stringstream ss(input);
    std::string token;
    while(std::getline(ss, token, ' '))
    {
        std::cout << token << "...";
    }

    std::cout << std::endl;
}

If you really want to use strtok, you might do something like this:

#include <cstring>
#include <string>
#include <iostream>
#include <vector>

int main(void)
{
    std::string input;
    std::getline(std::cin, input);

    std::vector<char> buffer(input.begin(), input.end());
    buffer.push_back('\0');

    char* token = strtok(&buffer[0], " ");
    for (; token; token = strtok(0, " "))
    {
        std::cout << token << "...";
    }

    std::cout << std::endl;
}

Remember, manually memory management is bad. Use a vector for arrays, and you avoid leaks. (Which your code has!)

Answer 5

GMan's answer is probably better and more purely c++. This is more of a mix which specifically uses strtok(), since I think that was your goal.

I used strdup()/free() since it was the easiest way to copy the string. In the question you were leaking memory since you'd malloc() with no matching free().

Also operator>> with the string will break on whitespace and so inappropriate for getting lines. Use getline() instead.

token.cpp

#include <iostream>
#include <string>
#include <cstring> /* for strtok() and strdup() */
#include <cstdlib> /* for free() */

int main(int argc, char * argv[]){
    std::string line;

    while(getline(std::cin, line)){
        char *pch = strdup(line.c_str());

        char *p = strtok(pch, " ");

        while(p){
            std::cout<<"Token: "<<p<<std::endl;
            p = strtok(NULL, " ");
        }

        std::cout <<"End of line"<<std::endl;
        free(pch);
    }
    return 0;
}

When you run this, you get what appears to be the correct result/

$ printf 'Hi there, I like tokens\nOn new lines too\n\nBlanks are fine'|./token
Token: Hi
Token: there,
Token: I
Token: like
Token: tokens
End of line
Token: On
Token: new
Token: lines
Token: too
End of line
End of line
Token: Blanks
Token: are
Token: fine
End of line