CODEWITHSUNDEEP
c
user14895999
user14895999

Reputation:

The following C code outputs a segmentation fault error which I can hardly understand why

#include <stdio.h>

void append(char* s, char n);
void splitstr(char* string);

int main()
{
    splitstr("COMPUTE 1-1");
    printf("\n");
    splitstr("COMPUTE 1+1");
    printf("\n");
    splitstr("COMPUTE 1*1");
    return 0;
}

void append(char* s, char ch) {
    while(*s != '\0'){
        s = s + 1;
    }
    *s = ch;
    s = s + 1;
    *s = '\0';
}

void splitstr(char* string){
    int count = 1;
    char* expression = "";
    while(*string != '\0'){
        if(count > 8){
            append(expression, *string);
            string = string + 1;
            count = count + 1;
        }else{
            string = string + 1;
            count = count + 1;
        }
    }
    printf("%s",expression);
}

Example Input and Output:
Input: COMPUTE 1+1
Output: 1+1
Input: COMPUTE 2-6
Output: 2-6

Originally, this code does not include stdio.h (I am doing this for testing on an online C compiler) because I am building an OS from scratch so I need to write all the functions by myself. I think the problem might be in the append function but I cannot find it.

Upvotes: 0

Views: 50

Answers (3)

John Bode
John Bode

Reputation: 123458

expression points to a string literal, and trying to modify a string literal leads to undefined behavior.

You need to define expression as an array of char large enough to store your final result:

char expression[strlen(string)+1]; // VLA

Since your result isn’t going to be any longer than the source string, this should be sufficient (provided your implementation supports VLAs).

Upvotes: 0

Kurt Weber
Kurt Weber

Reputation: 176

I think this line is the culprit:

append(expression, *string);

Notice how expression is declared:

char* expression = "";

In other words, expression consists of one byte, a single \0. Right away, we can see that append() won't work like you want it to--the while loop will never run, because *s is already \0.

But beyond that, the segfault likely happens at the bottom of append(). After the while loop, you unconditionally increment s and then write to the location it now points to. The problem is that this is a location that has never been allocated (since s is a reference to splitstr()'s expression, which is a single byte long). Furthermore, because expression is declared as a string constant, depending on your platform it may be placed in an area of memory marked read-only. Consequently, this is an attempt to write into memory that may not actually belong to the process and may also not be writable, raising the fault.

Upvotes: 0

Yehonatan harmatz
Yehonatan harmatz

Reputation: 449

instead of char* expression = ""; do char[MAX_expression_length+1] expression;

or use realloc in the append function

Upvotes: 1

Related Questions