Segmentation fault when assigning values to pointers in function

Question

Long time listener, first time caller.

I do apologize if this problem has already been addressed (I imagine that is has been covered extensively), but I've searched through many questions about pointers and other seemingly related topics, but I still am unable to solve my problem.

I'm currently writing a string library for class project, and when I am getting a segmentation fault error when I try this:

#include "str2107.h"
#include <stdio.h>

void lstrip(char *s) {

        char *p, *q;
        p = q = s;

        while (*p == ' ' && *p != '\0') {
             *p++;
        }

        while (*p != '\0') {
            *q = *p;   //this is where the actual segmentation fault occurs.
            p++;
            q++;
        }

        *q = '\0';
}

My main program looks like this:

#include <stdio.h>
#include <stdlib.h>
#include "str2107.h"


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

    char *z1 = "      weeee";
    printf("lstrip function\n");
    printf("%s\n",z1);
    lstrip(z1);
    printf("%s\n",z1);

    return 0;
}

Answer 1

z1 is pointing to a string literal and modifying a string literal is undefined behavior. Alternatively you can use the following declaration of z1 which will be modifiable:

char z1[] = "      weeee"; 

If we look at the C99 draft standard section 6.4.5 String literals paragraph 6 says(emphasis mine):

It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.

A few other points as WhozCraig points out this line:

while (*p == ' ' && *p != '\0') {

could be more succinctly written as:

while (*p == ' ' ) {

also you are using indirection here:

*p++;

but you don't actually use the resulting value, so you can change it to:

p++;

Answer 2

When you write char *z1 = " weeee"; then z1 is a pointer that points to a memory that is in the code part, so you can't change it.

If you change it char z1[] = " weeee"; then z1 is an array of chars that are on the stack and you can change it.

If you had wrote char const *z1 = "..."; then it would be a compilation error, which is better and preferable over segmentation fault.

Answer 3

char *z1 = "      weeee";

should be

const char *z1 = "      weeee";

because string literals are constant. Try something like:

const char *z1 = "      weeee";
char *z2 = strdup(z1);
lstrip(z2);

To call lstrip on a mutable string while still keeping z2 a char* (if it being a char[] isn't what you want).

Answer 4

As a lot of other people have already pointed out to you, when you write:

char *whatever = "some value";

This is much different than:

char *whatever = malloc(length("some value") + 1);
strcpy(whatever, "some value");

The difference being that something which is a literal (i.e. a string defined at compilation time) can be a constant, though I don't know if it is always or required to be constant. Since a constant should not be modified, and you're placing a '\0' character into the string, this is causing a segmentation fault.

There may be a lot of reasons for this, but the easy one I know of is that the program can optimize if it knows that every use of the same string is safe to reuse.

As an example:

char *whatever1 = malloc(length("some value") + 1);
strcpy(whatever1, "some value");
char *whatever2 = malloc(length("some value") + 1);
strcpy(whatever2, "some value");
if(whatever1 == whatever2) {
  printf("This should never happen!\n");
}

if("some value" == "some value") {
  printf("A good compiler should always do this!\n");
}