Segmentation Fault in string-splitting function

Question

I am not sure what is wrong with my code:

#include <stdio.h>
#include <string.h>

char* splitstr(char* str, int part, char search) {
    char* out;
    int i;
    int result = 0;
    for(i=0; str[i]!='\0'; i++) {
        if(str[i] == search) {
            result = 1;
            break;
        }
    }
    if(result == 0) {
        if(part == 1) {
            return str;
        } else {
            return "";
        }
    }
    int j;
    int k;
    if(part == 2) {
        for(j = 0; j < i; j++) {
            out[j] = str[j];
        }
        out[j] = '\0';
    } else {
        for(k = 0,j = i+1; j <= strlen(str)-1; j++, k++) {
            out[k] = str[j];
        }
        out[k] = '\0';
    }
    return out;
}   
}

int main() {
    printf("Starting program:\n");
    char* str = "Hello World!";
    char* a = splitstr(str, 1, ' ');
    char* b = splitstr(str, 1, ' ');
    printf("A is %s\n", a);
    printf("B is %s\n", b);
}

It returns the following output:

Starting program:
Segmentation Fault: 11

After debugging with gdb, I have found that the error is happening on line 30 (by using breakpoints), on the first iteration of the loop, when it tries to set out[0] (out[k]) to str[6] (str[j]). Why does this trigger a segmentation fault? I am just changing one character of a string to another!

note: the posted code has an extranious closing brace } after the statement: return out; — user3629249
– user3629249, Commented May 26, 2017 at 19:09

unwind · Accepted Answer · 2017-05-26 08:23:48Z

6

There is no string in out. It's an uninitialized pointer, you can't write through it since there is no memory allocated for your write. Doing so triggers undefined behavior, which is why your program is crashing.

You must allocate some, typically using malloc().

Also as a mere review-type note, the first loop is equivalent to:

const int result = strchr(str, search) != NULL;

answered May 26, 2017 at 8:23

unwind

402k64 gold badges492 silver badges620 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

user3629249 · Accepted Answer · 2017-05-26 19:23:21Z

the main problem is this line:

char* out;

only declares a pointer but doesn't set that pointer to point to any specific memory (that the application owns)

So this line:

out[j] = str[j];

is assigning values to some random location in memory.

Correct that problem via either:

char *out[ strlen( str ) +1 ];   // uses variable length array feature

or via:

char *out = malloc( strlen(str) +1 );
if( !out )
{ // malloc failed
    perror( "malloc failed" );
    exit( EXIT_FAILURE );
}

// implied else, malloc successful

Your compiler should have told you about this problem with a statement like:

filename.c:43:16: warning: 'out' may be used uninitialized in this function [-Wmaybe-uninitialized]

When compiling, always enable the warnings, then fix those warnings.

(for gcc, at a minimum use: -Wall -Wextra -pedantic I also use: -Wconversion -std=gnu11 )

If you use the -Wconversion option, then this will also be output by the compiler:

filename.c:51:30: warning: comparison between signed and unsigned integer expression [-Wsign-compare]

Collectives™ on Stack Overflow

Segmentation Fault in string-splitting function

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related