cutting a string when a character is found

Question

I wrote a function that cuts the string "hello world" to "hell" if a 'o' is found.

I keep getting a segmentation fault. I don't know where the mistake could be. Could anyone help? Thank you in advance.

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

char* cutString(char* str, char del){

    char *newstring =(char*) str;
    malloc(sizeof(char)*strlen(str));
    int i= 0;

    for(; newstring[i]!='\0'&&newstring[i]!=del;i++);

    if(i==strlen(newstring))
     printf("not found");
     else
     newstring[i]='\0';

    return newstring;
}


int main(){



    cutString("Hello World",'o');

    return 0;

}

`malloc(sizeof(char)*strlen(str));`? You need to learn what return values are and what they mean. You're creating a memory block with that line of code, and throwing it away because you don't save the return value. — Andrew Henle, Jan 07 '19 at 13:33

score 3 · Answer 1 · answered Jan 07 '19 at 13:34

3

There are two major problems with your code:

char *newstring =(char*) str makes newstring point to the old str. And since you pass a literal string (which is read only) you will have undefined behavior attempting to modify it.
malloc(sizeof(char)*strlen(str)); is a memory leak. And doesn't allocate space for the terminator.

The crash is probably because point one, when you attempt to modify the read-only string literal.

answered Jan 07 '19 at 13:34

Some programmer dude

400,186
35
402
621

so it should be like this `char *newstring= malloc(sizeof(char)*strlen(str));` – momonosuke Jan 07 '19 at 13:37
@momonosuke That's probably a good start, but also need some other work since then the memory will not contain the source string. And as mentioned there's not enough space for the terminator if the whole source string is copied. – Some programmer dude Jan 07 '19 at 13:39

manjy · Answer 2 · 2019-01-07T14:05:46.350

1

 newstring[i]='\0';

This line is invalid. Modifying string literals is undefined behavior. I would suggest check this out :segmentation fault when using pointer

A better solution would be to use arrays instead of pointers

edited Jan 07 '19 at 14:05

answered Jan 07 '19 at 13:47

manjy

109
1
2
12

the problem is I have to allocate new memory for the string . That's my assignment I read the article and I can see why my approach is incorrect – momonosuke Jan 07 '19 at 13:51

score 1 · Accepted Answer · answered Jan 07 '19 at 14:05

There are a number of problems in your code. The main problem is that you don't assign the return value from malloc to newstring. Besides that you need to malloc an extra byte for the string termination.

Further, your loop must copy characters from str into newstring.

In main you must assign the return value from the function to a char pointer variable to get hold of the new string.

Something like:

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

char* cutString(char* str, char del){

    char *newstring = malloc(strlen(str) + 1);  // malloc into newstring
    int i= 0;

    while (newstring[i]!='\0' && str[i] != del)  // Stop when a) no more chars in str or b) "del" is found
    {
        newstring[i] = str[i];     // Copy character from str to newstring
        ++i;
    }

    newstring[i]='\0';  // Terminate the string

    return newstring;
}


int main(){
    char* newstring = cutString("Hello World",'o');  // Save the returned value
    printf("%s\", newstring);
    free(newstring);
    return 0;
}

cutting a string when a character is found

3 Answers3