Splitting string into strings using strtok() not working

Question

I have a problem with regards to separating the contents of a string passed to a function. The function is called with a string like this:

ADD:Nathaniel:50

Where ADD will be the protocol name, Nathaniel will be the key, and 50 will be the value, all separated with a :.

My code looks like this:

bool add_to_list(char* buffer){

  char key[40];
  char value[40];
  int* token;

  char buffer_copy[1024];
  const char delim[2] = ":";

  strcpy(buffer_copy, buffer);

  token = strtok(NULL, delim);
  //strcpy(key, token);

  printf("%d",token);
  printf("%p",token);

  while(token != NULL){
    token = strtok (NULL, delim);
  }
  //strcpy(value, token);

  printf("%s", key);
  printf("%s", value);
  push(key, value);

  return true;
}

What I am trying to do is store each key and value in a separate variable, using strtok(). Note that I am trying to store the second and third values (Nathaniel and 50) not the first bit (ADD).

When I run the code, it gives me a segmentation fault, so I am guessing that I am trying to access an invalid memory address rather than a value. I just need to store the second and third bit of the string. Can anyone help please?

EDIT: I have changed the code to look like this:

bool add_to_list(char* buffer){

char *key, *value, *token;

const char *delim = ":";

token = strtok(buffer, delim);

//printf("%d",token);
printf("%s",token);

key = strtok(NULL, delim);
value = strtok(NULL, delim);

printf("%s", key);
printf("%s", value);
//push(key, value);

return true;
}

But I am still getting the same segmentation fault (core dumped) error

Answer 1

The first call to strtok() needs to provide the string to scan. You only use NULL on the repeated calls, so it will keep processing the rest of the string. SO the first call should be:

token = strtok(buffer_copy, delim);

Then when you want to get the key and value, you need to copy them to the arrays:

token = strtok(NULL, delim);
key = strcpy(token);
token = strtok(NULL, delim);
value = strcpy(token);

You don't need a loop, since you just want to extract these two values.

Actually, you don't need to declare key and value as arrays, you could use pointers:

char *key, *value;

Then you can do:

token = strtok(buffer_copy, delim);
key = strtok(NULL, delim);
value = strtok(NULL, delim);

Answer 2

Your main problem is that when you first call strtok, the first parameter should be the string you want to parse, so not:

strcpy(buffer_copy, buffer);
token = strtok(NULL, delim);

but

strcpy(buffer_copy, buffer);
token = strtok(buffer_copy, delim);

Additionally when you detect the tokens in your while loop, you are throwing them away. You want to do something at that point (or simply unroll the loop and call strtok three times).

Also:

const char* delim = ":";

would be a more conventional way of ensuring a NUL terminated string than:

const char delim[2] = ":";

Also consider using strtok_r not strtok as strtok is not thread-safe and horrible. Whilst you are not using threads here (it seems), you might as well get into good practice.