Memory error when copying parts of string using strtok

Question

So, I can see that strtok seems to be a much-despised function, but it is really well suited to my needs in this particular instance, and I'd like to avoid having to rewrite this whole function if that's possible. Of course, I'm open to accepting that strtok has to go if it really does.

Anyway, here's the problem I'm having. This function is reading in a user-specified string from a config file (that's what's happening in the first line). That string is a comma-separated list, containing pairs of numbers separated by a colon, like this:

    int:float, int:float, int:float

I want to store these values in some way that maps them to each other, with the ints being keys and the floats being values. The code that I have written to do this works just the way that I want it to as long as either the first int is only one digit, or multiple int:float pairs are present. If the string only has one int:float pair and the first int is two digits, then the function will execute a few times without a problem, but eventually garbage will be read into the index_token and ratio_token strings and the program will seg-fault. If I run the program in valgrind, this doesn't happen, so it must be some kind of a memory error. The string is getting read in from the file fresh each time this function is executed. When I print out const_ratios and ratios, they are as they should be every time.

Here's my code:

const char * const_ratios = m_world->GetConfig().NON_1_RESOURCE_RATIOS.Get();
cout << "Const_ratios: " << const_ratios;

char * ratios = new char[strlen(const_ratios)]; #make non const version of ratios
strcpy(ratios, const_ratios);                   #so that I can use strtok
cout << ", Ratios: " << ratios;

map<int, float> ratioMap;
char * ratio_tokens = strtok((char *)ratios, ",:");
while (ratio_tokens != NULL){

  char * index_token = new char[strlen(ratio_tokens)];
  strcpy(index_token, ratio_tokens);
  cout <<", Index token: " << index_token;

  ratio_tokens = strtok(NULL, ",:");
  char * value_token = new char[strlen(ratio_tokens)];
  strcpy(value_token, ratio_tokens);
  cout << ", Value token: " << value_token << endl;

  ratioMap[atoi(index_token)] = atof(value_token);
  ratio_tokens = strtok(NULL, ",:");

Does anyone have any idea why this might be happening? I assume it must have to do with strtok (perhaps in association with strcpy), but I can't see what I'm missing.

Answer 1

For C string, it needs the terminating null character at the end of the string, so for you code:

char * ratios = new char[strlen(const_ratios)]; 
strcpy(ratios, const_ratios);

The terminating null char is not appended the string 'ratios', which will cause problem for other functions. For example, function 'strcpy()', if you check the implementation of the function, it checks the terminating null char of the source string to decide if the copying process is completed. So if no terminating null char, it will cause memory errors.

So the code above should be like this:

int n = strlen(const_ratios) +1
char * ratios = new char[n]; 
strcpy(ratios, const_ratios);
ratios[n-1] = '\0'

Answer 2

You're not allocating enough memory. You allocate strlen(ratio_tokens) but then you copy in one more byte than that. This is one of the annoyances with C-style strings -- a C-style is always one byte larger than the number of characters in the string. Since you're coding in C++, why not use std::string?