Why is this code getting a segmentation fault?

Question

I am trying to write a function that deletes a char c from a string src, and I am getting a seg fault when I try to run it. Here is the function.

void removeChar(char *src, char c){
  int i, j = 0;
  int size;
  char ch1;
  char str1[100];

  size = strlen(src);

  for (i = 0; i < size; i++){
    if (src[i] != c){
      ch1 = src[i];
      str1[j] = ch1;
      j++;
    }
  }
  str1[j] = '\0';
  src = str1;
}

And here is the main function where I am calling it.

int main(int argc, char **argv){
    char *str = "Hello, world!\0";
    printf("%s\n", removeChar(str, 'l'));
}

you are using `void` and returning `value`. how? – Himanshu Nov 17 '14 at 06:16 — Himanshu, Nov 17 '14 at 06:16

score 2 · Answer 1 · answered Nov 17 '14 at 06:13

2

the return type of this function removeChar(str, 'l') is void not an char array and you are passing this to

printf("%s\n", removeChar(str, 'l'));

so here %s may give you the segmentation fault.

answered Nov 17 '14 at 06:13

smali

4,687
7
38
60

Vlad from Moscow · Answer 2 · 2014-11-17T08:28:05.280

You assigned pointer src by the address of the first element of a local array

  src = str1;

that will be destroyed after exiting the function. Moreover variable src is a local variable of the function so any changes of it do not influence the original pointer str.

Take into account that you may not change string literals. Any attempt to change a string literal results in undefined behaviour of the program.

Also the function has return type void and may not be used as an outputed object in function printf. Type void is an incomplete type. It has no values.

And there is no need to append explicitly terminating zero to a string literal as you did.

"Hello, world!\0"

String literals already have terminating zeroes. So you could write simply

"Hello, world!"

As I already answered this question then you can visit my personal forum where there is a realization of the corresponding valid function.

If to declare correctly the function like

char * removeChar( char *s, char c );

then the main will look the following way

int main(int argc, char **argv)
{
    char str[] = "Hello, world!";
    printf( "%s\n", removeChar( str, 'l' ) );
}

There's no dangling pointer here. The `src` parameter is passed by value so the assignment has no effect outside the function. — Blastfurnace, Nov 17 '14 at 06:18
@Blastfurnace You arer right! It is my mistake. I will change the post. — Vlad from Moscow, Nov 17 '14 at 06:21
@Blastfurnace It is funny that valid answers sometimes are down voted but invalid answers are upvoted.:) — Vlad from Moscow, Nov 17 '14 at 06:25

score 1 · Answer 3 · answered Nov 17 '14 at 06:17

1

You can print the string in the function itself! Then it works:

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

void removeChar(char src[], char c){
  int i, j = 0;
  int size;
  char ch1;
  char str1[100];

  size = strlen(src);

  for (i = 0; i < size; i++) {
    if (src[i] != c) {
      ch1 = src[i];
      str1[j] = ch1;
      j++;
    }
  }
  str1[j] = '\0';
  src = str1;

  printf("%s\n", src);

}

int main(int argc, char **argv) {

    char str[] = "Hello, world!";
    removeChar(str, 'l');

    return 0;

}

answered Nov 17 '14 at 06:17

Rizier123

58,877
16
101
156

Is there a way that I can use this function to simply change the original array, and then use it in the main function without actually printing it from the function itself? – Christian Nov 17 '14 at 06:23
You would have to pass the array by reference! – Rizier123 Nov 17 '14 at 06:24

score 1 · Answer 4 · edited May 23 '17 at 11:50

You have several bugs:

char *str = "Hello, world!\0";. Setting a non-constant pointer to point at a string literal is always wrong. Instead, declare the variable as const char *str. See this FAQ.
removeChar doesn't return anything so you can't pass it as a parameter to be printed by printf. Your compiler really should have complained here. Chances are that your compiler is misconfigured or you you aren't using it with all warnings enabled.
char str1[100]; You cannot use local variables and then try to pass the contents on to the caller. See this FAQ.
src = str1; doesn't do a thing, since src is only a local copy of the original pointer. With this assignment, you will not change the address of str in main. Which would have been a bug anyway, because of 3) above. You should rewrite your program so that is only uses src and no temporary array.

score 0 · Answer 5 · answered Nov 17 '14 at 07:12

Not have enough reputation to comment. So, I had to write this on answer:

As Vlad from Moscow pointed out,

`a local array do not exist after the function terminate`

I suggest you obey the same principle as of standard library functions. If you didn't already notice,none string.h function allocate memory for the user. You must allocate before call.

char *str = "Hello, world!\0";

The above code do not guarantee a modifiable memory. The compiler can set them in read only memory. You should use a array instead.

Why is this code getting a segmentation fault?

5 Answers5