Why am I getting a segmentation fault? C, basic code-CodePudding

I am getting a segmentation fault? Unlike other similar questions posted here, it doesn't seem to me that I'm trying to access an array index that is out of memory.

What I want the squeeze(char s1[], char s2[]) function to do, is to delete every char that occurs in s2 from s1 and print the result.

#include <stdio.h>
void squeeze(char s1[], char s2[]);

int main()
{
    squeeze("test string", "ts");
    
    return 0;
}

void squeeze(char s1[], char s2[])
{
    int i, j, k;

    for (k = 0; s2[k] != '\0'; k  ) {
        for (i=j=0; s1[i] != '\0'; i  )
            if (s1[i] != s2[k]) {
                s1[j] = s1[i];
                  j;
            }
        s1[j] = '\0';
    }
    printf("%s", s1);
}

Error occurs on the line s1[j] = s1[i];, when I tried to separate it into two lines and use another char as a placeholder, the segfault was on line with s1[j].

I also tried to use gdb, and I could read both s1[j] and s1[i].

Here is the segfault message from gdb:

Program received signal SIGSEGV, Segmentation fault.
squeeze (s1=0x555555556007 "test string", s2=0x555555556004 "ts") at main.c:18
18                  s1[j] = s1[i];

I've recently started learning C, and I have no idea what is the problem.

CodePudding user response：

The C standard does not define the behavior of attempting to write into the array of a string literal. "test string" is a string literal.

Instead of passing it directly to squeeze, define an array of char, initialize that array with the string literal, and pass that array to squeeze:

char buffer[] = "test string";
squeeze(buffer, "ts");

CodePudding user response：

In this call

squeeze("test string", "ts");

you are trying to change a string literal. Though in C opposite to C string literals have non-constant character array types nevertheless any attempt to change a string literal results in undefined behavior.

You should declare a character array and initialize it with the string literal as for example

char s1[] = "test string";
squeeze( s1, "ts");

Also the function should be declared like

char * squeeze( char s1[], const char s2[] );

And the used approach is inefficient.

The function can be defined the following way as shown in the demonstrative program below

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

char * squeeze( char s1[], const char s2[] )
{
    if ( *s2 )
    {
        char *p = s1, *q = s1;

        do
        {
            if ( *p == '\0' || strchr( s2, *p ) == NULL )
            {
                if ( p != q ) *q = *p;
                  q; 
            }
        } while ( *p   );
    }

    return s1;
}

int main( void )
{
    char s1[] = "test string";

    puts( squeeze( s1, "ts" ) );
}

The program output is

e ring