strsep() causing Segmentation fault

Question

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.

what is `commands`? Please post an **entire program** that reproduces the problem, including the call to this function. — Antti Haapala -- Слава Україні, Sep 13 '20 at 12:51
You're assigning a pointer to characters to `commands[i]` which is a *char*. Chances are you're doing something that overwrites the pointer that strsep is acting upon. — Antti Haapala -- Слава Україні, Sep 13 '20 at 12:53
I can't try this now so I won't post an answer, but it looks like you'd want to get rid of the first 4 lines (`sp` and `temp` variables), and just call it as `strsep(&string, character)`. Also check for `string` being set to `null`. — user2740650, Sep 13 '20 at 12:55
AFAICS, the seg fault comes from writing to string literals. `strsep()` tries to modify `Hello World` (at least if you passed a space instead of an `@` sign), and then tries to modify `command` too. I think you're misusing `strsep()` too. I would expect to pass `&string` as the first argument, rather than `sp`, even though the value in `sp` is `&string`. At least, that's a more conventional usage of it. — Jonathan Leffler, Sep 13 '20 at 15:51
Yeah sorry for the mess I was looking at https://docs.oracle.com/cd/E19048-01/chorus401/806-4422/6jdgkltde/index.html as an example for how to use strsep — Justin Haindel, Sep 13 '20 at 15:55
What output do you expect from `split("Hello World", "@", "&")`? There are no modifiable parameters, and the return value is (in the revised code) always `0`. Please create an MCVE ([Minimal, Complete, Verifiable Example](https://stackoverflow.com/help/mcve) — or MRE or whatever name SO now uses) or an SSCCE ([Short, Self-Contained, Correct Example](http://sscce.org/)). That will add a minuscule `main()` function that shows how you're calling the function and processing the output from the function. Splitting `"Hello World"` on `&` seems like you'd only have one token in the result. — Jonathan Leffler, Sep 13 '20 at 15:58
I would expect just "Hello World" to be stored in commands[0] since there is no &. — Justin Haindel, Sep 13 '20 at 16:13

chqrlie · Accepted Answer · 2020-09-13T21:37:05.633

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.

The memory allocated via `temp` is leaked — there's no guarantee that the first token starts at the beginning of the string (in general; the sample data is safe on that score). — Jonathan Leffler, Sep 13 '20 at 15:46
In general it worked however it did not solve my new problem where every index of commands is out of bounds. — Justin Haindel, Sep 13 '20 at 15:54
@JonathanLeffler: I fixed the memory leak issue. I noticed it just after posting, but I went out of coverage :) — chqrlie, Sep 13 '20 at 21:38
@JustinHaindel: does your function still have the incorrect prototype? — chqrlie, Sep 13 '20 at 21:44

strsep() causing Segmentation fault

1 Answers1