String separator, similar to strtok I made a string separator below. It's like strtok, but it separates based on entire strings instead of single char delimiters. Any comments/answers on how I could improve the performance or readability would be great. I'm also leaking some memory according to valgrind, but that is only a minor concern for my use case.

It takes two char arrays, a buffer(string?) and a separator. It returns an array of strings containing all of the separated strings. For example, if the buffer is foobar and the separator is foo, it'll return {"bar", NULL}. But if I'm passed foobarfoobarfoobar and the separator is bar, it'll return {"foo", "foo", "foo", NULL}.

Edit: Comments added to clarify intended functionality.

#include &lt;stdio.h&gt;
#include &lt;string.h&gt;
#include &lt;stdlib.h&gt;

char **sepstr(const char *buf, const char *sep){
    int i = 0, j = 0, k = 0, l = 0, stringCount = 0;

    /* If separator &gt; buffer, return NULL */
    while (buf[i]) i++;
    while (sep[j]) j++;
    if (j &gt; i) return NULL;

    char **strings = malloc(0);

    /* While we're not at the end of the buffer */
    while (*(buf+k)){
        /* If the characters match, check to see if it is the separator */
        if (*(buf+k) == *(sep)){
            for (l = 0; l &lt; j; l++){
                /* If it isn't the separator, break */
                if (*(buf+k+l) != *(sep+l)) break;
                /* If it is the separator and the separator isn't the
                    beginning, add buf to buf+k bytes as a string */
                if (l == j-1 &amp;&amp; k != 0){
                    strings = realloc(strings, (stringCount+1)*sizeof (char *));
                    strings[stringCount] = malloc(k+1);
                    memcpy(strings[stringCount++], buf, k);
                    buf += (j + k), k = -1;
                /* If it is the separator, but it's the beginning
                    of the string, skip it */
                } else if (l == j-1 &amp;&amp; k == 0) {
                    buf += j, k = -1;
                }
            }
        }
        k++;
    }

    /* Add a string for the left over bytes if sep isn't the end */
    if (i != k &amp;&amp; *(buf)){
        while (buf[l]) l++;
        strings = realloc(strings, (stringCount+1) * sizeof (char *));
        strings[stringCount] = malloc(l+1);
        memcpy(strings[stringCount++], buf, l);
    }

    /* Append NULL to array of strings */
    strings = realloc(strings, (stringCount+1) * sizeof (char *));
    strings[stringCount] = malloc(sizeof NULL);
    strings[stringCount] = NULL;

    return strings;
}

int main(){
    const char buffer[] = "foobarfoobarfoobar";
    const char separator[] = "bar";
    char **strings = sepstr(buffer, separator);

    while(*strings){
        printf("%s\n", *(strings));
        free(*(strings++));
    }

    free(strings);

    return 0;
}
I'm also leaking some memory according to valgrind, but that is only a minor concern for my use case.


Hmm, I don't really agree. Leaking memory is harmless only if you know the cause (*), and simply choose to not spend time in fixing it. In your case, the cause (for one part, see below) is the print loop at the end of the main:

while(*strings){
    printf("%s\n", *(strings));
    free(*(strings++));  // incrementing strings here
}

free(strings); // string no longer points to the initially allocated bloc: UB


Whether it matters is your problem, but you should not reproduce this code. BTW, it caused a crash on my VS2017...

Now for the function itself:


char** strings = malloc(0); Nothing really bad here because it will be reallocated later, but as it is more of a trick, it really deserves a comment explaining the reason for that line
Your naming convention for local variables (int i = 0, j = 0, k = 0, l = 0, stringCount = 0;) does not help to understand their meaning. After analysis I would prefer:

i -&gt; buflen
j -&gt; seplen
k -&gt; buf_ix  // or whatever you want saying it is intended to be an index of buf
l -&gt; sep_ix

the strings in strings are not null-terminated. This is really bad because there is no way to find the used length: you really should add that terminating null - anyway, you have reserved memory for it...
An additional memory leak when setting last element of strings to NULL:

/* Append NULL to array of strings */
strings = realloc(strings, (stringCount+1) * sizeof (char *));
strings[stringCount] = malloc(sizeof NULL);    // hmm... a dynamic allocation...
strings[stringCount] = NULL;                   // immediately and definitively lost !


this code should become:

/* Append NULL to array of strings */
strings = realloc(strings, (stringCount+1) * sizeof (char *));
strings[stringCount] = NULL;

you should test malloc and realloc return values. Allocation error are uncommon nowadays, but if this code was to run under stressed condition very bad things would occur, because you would be using a null pointer which is Undefined Behaviour (other details on @TobySpeight's answer).




(*) Even if you had identified that the code in main caused a memory leak, it could have masked the other memory leak inside the function the deserved to be fixed. That's the reason why it is bad to leave a memory leak in a program, and even in a test only one!