Random behavior of an array of char arrays

Question

I have an array of character arrays that is split based on a pipe ('|') operator (example below) and the function I am using to create this array seems to work on occasion, and then on occasion, it will create the array then abort and give me one of two different errors.

I am not sure what I am doing wrong? Particularly I am not sure why it creates successfully every time but then seems to break after creation about half the time, regardless of the input.

Example array:

"here is | an example | input" = {"here is", "an example", "input"}

Errors:

Error in './msh': malloc(): memory corruption (fast): 0x000...

Error in './msh': free(): invalid pointer: 0x0000....

Code:

char** allArgs = new char*[100];
void createArgArrays(const char* line) {
   char* token;
   token = strtok((char*)line, "|");
   int i = 0;
   while(token != NULL) {
      allArgs[i] = token;
      i++;
      token = strtok(NULL, "|");
   }
}

Where I call the code:

string input;
getline(cin, input);
createArgArrays(input.c_str());

Any insight/help is greatly appreciated.

Why not use a `std::vector>`? You are not doing your array allocation correctly, and you should use appropriate C++ classes, that's what they're there for — Cory Kramer, Apr 15 '18 at 18:40
The `(char*)` cast is ugly, wrong and dangerous. You're promising the caller not to modify the thing they gave you, then modify it... — Mat, Apr 15 '18 at 18:41
From Wikipedia: "The vertical bar ( | ) is a computer character and glyph with various uses in ...". This char is not indicating a 'pipe' in this usage. — 2785528, Apr 15 '18 at 18:42
There are so many problems here, it's hard to decide where to start. I think you need to start [with a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — Sam Varshavchik, Apr 15 '18 at 19:02
C++? Your code looks more like C to me. Consider getting a [good C++ book](https://stackoverflow.com/q/388242/9254539) instead of courses that teach bad C and call it C++. — eesiraed, Apr 15 '18 at 19:03
If you want to write to your string buffer you can use `&input[0]` rather than `input.c_str()`. — Galik, Apr 15 '18 at 19:08
Thanks for the advice, I haven't used C++ for 2 years, however, I use C on a regular basis so I think that is why it is looking more like C. And I practically never use Char arrays... I am going to try to implement it as a vector instead and then create a char array that copies the string vector. I did this elsewhere in my code and it seems to work. — Cassie, Apr 15 '18 at 19:56

score 2 · Answer 1 · answered Apr 15 '18 at 18:41

c_str() returns a const char *. strtok() modifies the string it refers to.

Per http://www.cplusplus.com/reference/string/string/c_str/:

c++98

A program shall not alter any of the characters in this sequence.

Don't cast away const to force things to "work".

score 0 · Answer 2 · answered Apr 15 '18 at 20:08

A few things:

The C++ way is sometimes different than the C way.

Andrew Henle's point about casting should be carved into stone tablets.

If you really want to use a C function, try walking the string using strchr().

Also, try something like std::vector<std::string> (see std::vector::push_back) to store your string chunks - it'll be a bit cleaner and avoids an arbitrary cap on the size of allArgs.

Another thing you could look at is boost::split(), which probably does exactly what you want anyway.

Random behavior of an array of char arrays

2 Answers2