18

Can anyone explain why I am getting segmentation fault in the following example?

#include <stdio.h>
#include <string.h>

int main(void) {
  char *hello = "Hello World, Let me live.";
  char *tokens[50];
  strtok_r(hello, " ,", tokens);
  int i = 0;
  while(i < 5) {
    printf("%s\n", tokens[i++]);
  }
}
codaddict
  • 445,704
  • 82
  • 492
  • 529

6 Answers6

25

Try this:

#include <stdio.h>
#include <string.h>

int main(void) {
        char hello[] = "Hello World, Let me live."; // make this a char array not a pointer to literal.
        char *rest; // to point to the rest of the string after token extraction.
        char *token; // to point to the actual token returned.
        char *ptr = hello; // make q point to start of hello.

        // loop till strtok_r returns NULL.
        while(token = strtok_r(ptr, " ,", &rest)) {

                printf("%s\n", token); // print the token returned.
                ptr = rest; // rest contains the left over part..assign it to ptr...and start tokenizing again.    
        }
}
/*
Output:
Hello
World
Let
me
live.
*/
missingfaktor
  • 90,905
  • 62
  • 285
  • 365
codaddict
  • 445,704
  • 82
  • 492
  • 529
  • This example gives me a segfault on the printf line. gdb print token shows 0xffffffffffffdad0
    Also get these two warning when I compile proj1.c:33:2: warning: implicit declaration of function ‘strtok_r’ [-Wimplicit-function-declaration] proj1.c:33:14: warning: assignment makes pointer from integer without a cast [enabled by default]
    – Nathan Schwermann Oct 01 '12 at 00:48
  • 1
    Sorry I'm a newbie, why shouldn't it be `char *ptr = Hello;` with a capital H? Also, Alok's answer says the first time the first parameter needs to "tokenized" and then subsequent calls it needs to be NULL, but it appears your example just calls it the one way in the while loop? Thanks for the code btw – SSH This Dec 17 '12 at 22:48
  • `*ptr = hello;` because `hello` refers to `char hello[]...`. – MrHIDEn Jan 09 '15 at 14:06
17
  • You need to call strtok_r in a loop. The first time you give it the string to be tokenized, then you give it NULL as the first parameter.
  • strtok_r takes a char ** as the third parameter. tokens is an array of 50 char * values. When you pass tokens to strtok_r(), what gets passed is a char ** value that points to the first element of that array. This is okay, but you are wasting 49 of the values that are not used at all. You should have char *last; and use &last as the third parameter to strtok_r().
  • strtok_r() modifies its first argument, so you can't pass it something that can't be modified. String literals in C are read-only, so you need something that can be modified: char hello[] = "Hello World, Let me live."; for example.
Alok Singhal
  • 93,253
  • 21
  • 125
  • 158
  • 1
    Thanks for the answer. I wish SO allowed marking multiple answers as correct. :) – Phulore R - Profile 2 Feb 09 '10 at 07:00
  • @Scrub: glad to be of help. Make sure you understand my second point above (about `char *tokens[50];` being equivalent to `char **` when passed to a function). – Alok Singhal Feb 09 '10 at 07:04
  • 1
    SAVED me; +1 for reminding me, that param 1 cannot be `const char *`. For others: Don't feed in data from the .RODATA section. Or: try to see if it works with `strcpy`'ing to a temp buffer beforehand. – Joel Jun 25 '21 at 06:27
5

A bunch of things wrong:

  1. hello points to a string literal, which must be treated as immutable. (It could live in read-only memory.) Since strtok_r mutates its argument string, you can't use hello with it.

  2. You call strtok_r only once and don't initialize your tokens array to point to anything.

Try this:

#include <stdio.h>
#include <string.h>

int main(void) {
  char hello[] = "Hello World, Let me live.";
  char *p = hello;
  char *tokens[50];
  int i = 0;

  while (i < 50) {
     tokens[i] = strtok_r(p, " ,", &p);
     if (tokens[i] == NULL) {
        break;
     }
     i++;
  }

  i = 0;
  while (i < 5) {
    printf("%s\n", tokens[i++]);
  }

  return 0;
}
jamesdlin
  • 81,374
  • 13
  • 159
  • 204
3

strtok_r tries to write null characters into hello (which is illegal because it is a const string)

ornerylawn
  • 524
  • 1
  • 4
  • 9
2

You have understood the usage of strtok_r incorrectly. Please check this example and documentation

And try & see this:

#include <stdio.h>
#include <string.h>    

int main(void)
{
    char hello[] = "Hello World, let me live.";

    char *tmp;
    char *token = NULL;
    for(token = strtok_r(hello, ", ", &tmp);
        token != NULL;
        token = strtok_r(NULL, ", ", &tmp))
    {
        printf("%s\n", token);
    }

    return 0;
}
Anssi
  • 518
  • 4
  • 8
0

I think it might be the char *tokens[50]; because you are declaring it a pointer when it is already a pointer. An array is already a pointer upon declaration. You mean to say char tokens[50];. That should do the trick.

SapphireSun
  • 9,170
  • 11
  • 46
  • 59