0

I am having trouble figuring out why I keep getting a segfault when trying to run a small test. The idea is that I send in a fraction as a string such as "1/4" in this case and the string_to_fraction is supposed to tokenize the string by getting rid of the "/" and setting the numerator value to 1 and the denominator value to 4 then returning it.

typedef struct {
    int numer;
    int denom;
}Fraction;

Fraction string_to_fraction(const char *S);

main(){
    const char* strn = "1/4";
    Fraction myFrac;
    myFrac = string_to_fraction(strn);
    return 0;
}

Fraction string_to_fraction(const char *S)
{
    Fraction result = {0,1};
    char *p;
    char n[100] = {}, d[100] = {};
    p = strtok(S,"/");
    if(p == NULL){
        printf("String of improper format");
        return result;
    }
    strcpy(n,p);
    p = strtok(NULL,"/");
    strcpy(d,p);
    result.numer = n;
    result.denom = d;
    return result;
}

the segfault occurs at the first instance of strtok, e.g. p = strtok(S,"/"); but I have no idea why this is happening, I have tried passing many different things into the string_to_fraction function but it always segfaults. Any insight is much appreciated as all the other issues people had with strtok causing a segfault did not really seem to pertain to my situation as far as I can tell.

edit: forgot to include my declaration to the fraction_to_string function on here, that is now fixed. I did it in my code but this code is split by a lot of junk so i just re-wrote a lot of it here.

NoxNostriSilenti
  • 3
  • 1
  • possible duplicate of [strtok wont accept: char \*str](http://stackoverflow.com/questions/2529834/strtok-wont-accept-char-str) – Retired Ninja Apr 11 '15 at 22:52
  • 1
    Calling `strtok` modifies the string you passed to it, however S is passed from a character literal which are allocated on the data segment and are immutable (since it's a literal). You need to make a copy of S. – Benjamin Gruenbaum Apr 11 '15 at 22:54
  • so something like `char* newS; strcpy(newS,S);` then call the strtok as `p = strtok(newS,"/");`? – NoxNostriSilenti Apr 11 '15 at 22:57
  • Wait no, according to another thing here that would have the same problem as it apparently cannot modify char* or something, so then would i instead need something along the lines of: `char newS[100] = {}; strcpy(newS,S);`? – NoxNostriSilenti Apr 11 '15 at 23:01
  • No, since you haven't allocated any space for `newS`. You could use `strdup` or allocate the space you're trying to copy it to. – Retired Ninja Apr 11 '15 at 23:01
  • so it is as simple as doing `char *newS; newS = strdup(S);` before the strtok then calling strtok on newS rather than S? – NoxNostriSilenti Apr 11 '15 at 23:08
  • since the posted code is returning an instance of a struct rather than a pointer to a struct, the compiler will allocate some 'hidden' memory then use two calls to memcpy(). the first call to copy the returned struct to the hidden memory and the second call to copy the hidden memory to the callers struct instance. thats is a lot of extra/hidden code, lost memory, etc. Much better to pass a second parameter and have the called function copy the data to the callers's struct definition. – user3629249 Apr 12 '15 at 20:53

1 Answers1

2

Yes, as Benjamin said - string literals are allocated in read-only memory and strtok modifies the string itself, e.g. stores '\0' character in place of delim.

Compiler warning

warning: passing argument 1 of ‘strtok’ discards ‘const’ qualifier from pointer target type

is the hint here, that something is wrong.

Here's how to make a copy:

replace

const char* strn = "1/4";

with 

char buf[8192];
snprintf(buf, 8192, "1/4");
const char* strn = buf;

...or with:

char *strn = strdup("1/4");
...
free(strn);//<<-- but then your would need to free memory in the end of program

There is one more problem with your code - you use strings as integers. C won't convert them automatically:

result.numer = n;
result.denom = d;

replace with:

result.numer = atoi(n);
result.denom = atoi(d);
fukanchik
  • 2,811
  • 24
  • 29
  • Ah thanks, I will try to make a few changes using this and see if i can get it fixed. – NoxNostriSilenti Apr 11 '15 at 23:09
  • I got it working! Thank you very much to you (fukanchik) as well as Retired Ninja. i ended up using `char *scpy = strdup(S);` then calling `p = strtok(scpy,"/");` then calling `free(scpy);` just before the return. The strings not being ints was something I didn't even think about before, so mentioning atoi saved me from a lot of future suffering. – NoxNostriSilenti Apr 11 '15 at 23:15