Problem with strtok and segmentation error

Question

I have two helper functions for splitting strings in decimal format, i.e. "23.00", "2.30"

Consider this:

char price[4] = "2.20"; unsigned getDollars(char *price) { return atoi(strtok(price, ".")); } unsigned getCents(char *price) { strtok(price, "."); return atoi(strtok(NULL, ".")); } 

Now, when I run below, I get a segmentation error:

 printf("%u\n", getDollars(string)); printf("%u\n", getCents(string)); 

However, when I run them separately without following another, they work fine. What am I missing here? Should I do some strtok reset ??

My decision:

Thanks to the knowledge about strtok obtained from the answer I selected below, I changed the implementation of helper functions to first copy the passed in line, protecting the original line and preventing this problem:

  #define MAX_PRICE_LEN 5 /* Assumes no prices goes over 99.99 */ unsigned getDollars(char *price) { /* Copy the string to prevent strtok from changing the original */ char copy[MAX_PRICE_LEN]; char tok[MAX_PRICE_LEN]; /* Create a copy of the original string */ strcpy(copy, price); strcpy(tok, strtok(copy, ".")); /* Return 0 if format was wrong */ if(tok == NULL) return 0; else return atoi(tok); } unsigned getCents(char *price) { char copy[MAX_PRICE_LEN]; char tok[MAX_PRICE_LEN]; strcpy(copy, price); /* Skip this first part of the price */ strtok(copy, "."); strcpy(tok, strtok(NULL, ".")); /* Return 0 if format was wrong */ if(tok == NULL) return 0; else return atoi(tok); } 

Answer 1

Since strtok() modifies the input string, you run into problems when you cannot find the separator in the getCents() function after calling getDollars() .

Note that strtok() returns a null pointer when it cannot find the delimiter. Your code does not verify that strtok() found what it was looking for, which is always dangerous.

---

Your update to the question indicates that you learned at least some of the dangers (angry?) strtok() . However, I would suggest that only strchr() would use the best solution.

First, we can notice that atoi() in any case stop converting to ' . ', so we can simplify getDollars() to:

 unsigned getDollars(const char *price) { return(atoi(price)); } 

We can use strchr() - which does not modify the string - to find '.' , and then process the text after it:

 unsigned getCents(const char *price) { const char *dot = strchr(price, '.'); return((dot == 0) ? 0 : atoi(dot+1)); } 

Pretty much simpler, I think.

---

Another result: suppose line 26.6; you will have to work harder than the revised getCents() just above to get this to return 60 instead of 6. Also, given 26.650, it will return 650, not 65.

Answer 2

It:

 char price[4] = "2.20"; 

excludes nul terminator at price . I think you want this:

 char price[5] = "2.20"; 

or better:

 char price[] = "2.20"; 

So, you run away from the end of the buffer a second time when you try to get a token from price . You are just lucky that getCents() not segfault every time you run it.

And you should almost always make a copy of the string before using strtok on it (to avoid the problem Jonathan Leffler pointed out).