CODEWITHSUNDEEP
csegmentation-faultstrsep
Justin Haindel
Justin Haindel

Reputation: 23

strsep() causing Segmentation fault

I am having a problem with my program in which I am getting a segmentation fault from strsep() which was gotten from GDB and has the error message

Program received signal SIGSEGV, Segmentation fault.
0x00002aaaaad64550 in strsep () from /lib64/libc.so.6

My code is as follows:

int split(char *string, char *commands, char *character) {
    char **sp = &string;
    char *temp;
    temp = strdup(string);
    sp = &temp;
    for (int i = 0; i < 100; i++) {
        commands[i] = strsep(sp, character);
        if (commands[i] == '\0') {
            return 0;
        }
        if (strcasecmp(commands[i], "") == 0) {
            i--;
        }
        printf("%d", i);
    }
    return 0;
}

Any help would be greatly appreciated as I have spent hours trying to solve this problem

The arguments for the function are ("Hello World", "@", "&")

EDIT

So I have managed to get rid of the segment fault by changing the code to

int split(char* string, char* commands, char* character) {
        for(int i = 0; i < 100; i++) {
                commands[i] = strsep(&string, character);
                if(commands[i] == '\0') {
                        return 0;
                }
                if(strcasecmp(&commands[i], "") == 0) {
                        i--;
                }
        }
        return 0;
}

However now I have a new problem with commands returning a null array where every index is out of bounds.

EDIT 2

I should also clarify on what I am trying to do a bit so essentially commands is of type char* commands[100] and I want to pass it into the function when then modifies the original pointer array and store say `"Hello World"' into commands[0] then I want to modify this value outside the function.

Upvotes: 1

Views: 579

Answers (1)

chqrlie
chqrlie

Reputation: 144780

Your usage of commands is inconsistent with the function prototype: the caller passes an array of 100 char*, commands should be a pointer to an array of char *, hence a type char **commands or char *commands[]. For the caller to determine the number of tokens stored into the array, you should either store a NULL pointer at the end or return this number or both.

Storing commands[i] = strsep(...) is incorrect as commands is defined as a char *, not a char **.

It is surprising you get a segmentation fault in strsep() because the arguments seem correct, unless character happens to be an invalid pointer.

Conversely you have undefined behavior most likely resulting in a segmentation fault in strcasecmp(commands[i], "") as commands[i] is a char value, not a valid pointer.

Here is a modified version:

// commands is assumed to point to an array of at least 100 pointers
// return the number of tokens or -1 is case of allocation failure
int split(const char *string, char *commands[], const char *separators) {
    char *dup = strdup(string + strcspn(string, separators));
    if (temp == NULL)
        return -1;
    char *temp = dup;
    char **sp = &temp;
    int i = 0;
    while (i < 99) {
        char *token = strsep(sp, separators);
        if (token == NULL) // no more tokens
            break;
        if (*token == '\0') // ignore empty tokens
            continue;
        commands[i++] = token;
    }
    commands[i] = NULL;
    if (i == 0) {
        free(dup);
    }
    return i;
}

The memory allocated for the tokens can be freed by freeing the first pointer in the commands array. It might be simpler to duplicate these tokens so they an be freed in a more generic way.

Upvotes: 1

Related Questions