Segfault while accessing memory malloc'd in function

Question

I'm trying to write a function that takes in a path (char *) and splits it into an array of strings based around the '/' delimiter. Simplified code below :

int split_path(char * path, char ** out) {
    out = NULL;
    char * token = strtok(path, "/");
    int count = 0;

    while(token) {
        out = realloc(out, sizeof(char*) * (++count));
        out[count-1] = malloc(sizeof(char) * strlen(token)+1);
        strcpy(out[count-1], token);
        fprintf(stderr, "%s\n", out[count-1]);

        token = strtok(NULL, "/");
    }   

    out = realloc(out, sizeof(char*) * (count+1));
    out[count] = NULL;

    return count;
}

int main(int argc, char * argv[]) {
    char path[] = "/home/pirates/are/cool/yeah";

    char ** out;

    int count = split_path(path, out);

    fprintf(stdout, "count: %d\n", count);
    fprintf(stderr, "1st: %s\n", out[0]); // segfaults here
    return 0;
}

All of the print statements in the split_path function print perfectly, the output looks like this :

count: 1, string: home
count: 2, string: pirates
count: 3, string: are
count: 4, string: cool
count: 5, string: yeah
count: 5
1st: ./a.out
[1]    5676 segmentation fault (core dumped)  ./a.out

But for some reason when I get back to the main function the double-char-array is no longer valid. I thought that it might be because it was pointing to memory declared in that split_path function but I'm doing strcpy to get the strings into it so it shouldn't be pointing back to memory that is local to that function. Any help is greatly appreciated.

I'll give it a try but I'm not sure why that would make much of a difference. — John Allard, May 29 '16 at 21:45
what @BLUEPIXY said... or use char*** so out actually gets changed and returned. — thang, May 29 '16 at 21:49
well that worked, but why? out is a pointer so why wouldn't it get changed? Would I need another level of pointer, like char**? — John Allard, May 29 '16 at 21:51
You *aren't* accessing the memory allocated in the function. — user253751, May 29 '16 at 22:53
when calling `malloc()` or `realloc()`, always check (!=NULL) the returned value to assure the operation was successful. When calling `realloc()`, do not directly assign the returned value to the target pointer, Because if `realloc()` fails, then the original pointer is lost, resulting in a memory leak. Instead, assign to a 'temp' variable, check that variable for NULL, and if not NULL, then assign the target variable from the 'temp' variable. — user3629249, May 29 '16 at 23:51
regarding this kind of line: `out = realloc(out, sizeof(char*) * (++count));` this is only changing the value of the parameter, not the variable back in `main()`, The line should be similar to: `*out = realloc(*out, sizeof(char*) * (++count));`. Similar considerations exist for every reference to `out` in the `split_path()` function. — user3629249, May 29 '16 at 23:55

Remy Lebeau · Accepted Answer · 2016-05-29T22:04:19.880

You are mismanaged the out parameter. The out variable in main() is never assigned a valid memory address, thus the segfault. The out parameter in split_path() never updates the out variable in main(). You need to pass the address of the variable to split_path() so it can update the variable, and access the memory that the variable points to.

Also note that strtok() modifies the string it is parsing, so you should make a copy and then parse the copy so the original does not get destroyed. Otherwise, consider using strchr() instead of strtok().

Try something more like this instead:

int split_path(char * path, char *** out) {
    *out = NULL;
    char * tmp = strdup(path);
    if (!tmp) { ... }
    char * token = strtok(tmp, "/"');
    int count = 0;
    char ** newout;

    while (token) {
        newout = realloc(*out, sizeof(char**) * (++count));
        if (!newout) { ... }
        *out = newout;
        (*out)[count-1] = malloc(sizeof(char) * (strlen(token)+1));
        if (!(*out)[count-1]) { ... }
        strcpy((*out)[count-1], token);
        fprintf(stderr, "%s\n", token);

        token = strtok(NULL, "/");
    }   

    newout = realloc(*out, sizeof(char**) * (count+1));
    if (!newout) { ... }
    *out = newout;
    (*out)[count] = NULL;

    free (tmp);
    return count;
}

int main(int argc, char * argv[]) {
    char path[] = "/home/pirates/are/cool/yeah";

    char ** out;

    int count = split_path(path, &out);

    fprintf(stdout, "count: %d\n", count);
    fprintf(stderr, "1st: %s\n", out[0]); // segfaults here

    free (out);
    return 0;
}

And don't forget error handling. I've left it out of this example for brevity, but you should not leave it out of your real code.

it works! thank you, I realize now that if I'm wanting to maintain a reference to a 2d array of memory I need a triple pointer, just like if I want a reference to a single int I need an int pointer. Makes sense. — John Allard, May 29 '16 at 21:53

Segfault while accessing memory malloc'd in function

1 Answers1