C - Segmentation fault at strcpy()

Question

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

char *vStrs[] = {"Max", "Moritz", "Bolte", "Hans Huckebein", "Helene", "Antonius", "Boeck", "Maecke", "Lempel", "Schlich"};

int main()
{
    int num = sizeof(vStrs) / sizeof(vStrs[0]);
    int len = sizeof(vStrs[0]);
    char exchnge[len];
    char vBuf[128];
    char *ndata;
    int i, j;

    for(i=0; i<num-1; i++)
    {
        for(j=i+1; j<num; j++)
        {
            if(strcmp(vStrs[j], vStrs[i]) < 0)
            {
                strcpy(exchnge, vStrs[j]);
                strcpy(vStrs[j], vStrs[i]);
                strcpy(vStrs[i], exchnge);
            }
        }
    }

    for(i=0; i<num; i++)
        printf("%s\n", vStrs[i]);

    return 0;
}

Hey guys,

does anyone know why I get a segmentation fault at line strcpy(vStrs[j], vStrs[i]);?

I have an array of strings and want to sort it. But I get a segmentation fault. The strcpy()-function above that works. What is wrong?

Probably it's obvious but I don't get it.

Thankyou!

You cannot write to the array. String literals have type const char* — Bjorn A., Feb 09 '18 at 12:32
What you try to do? Copy one constant in the place of another? — i486, Feb 09 '18 at 12:32
Additionally, this line will also cause a seg fault strcpy(exchnge, vStrs[j]); eventually. The length of exchnge is only 3. Many of your strings are longer than that. — jakebower, Feb 09 '18 at 12:33
`= sizeof(vStrs[0]);` gives you the size of a _pointer_. So the buffer "exchnge" is too small, causing the crash. Overall, you seem to be programming through trial & error. That will _never_ work, you have to actually know what every line in your program does. — Lundin, Feb 09 '18 at 12:34
@Martin Chekurov: It's not a matter of length; modifying string literals is UB, even if both strings had the same length. — Stephan Lechner, Feb 09 '18 at 12:55
@BjornA.: String literals are `const` **in C++**. They are *not* in C! They are non-writeable, but their type is still just `char *`. If you enable `-Wwrite-strings`, you *make* them `const` (and the compiler will thusly emit a warning), but as far as the language standard is concerned... — DevSolar, Feb 09 '18 at 13:16
@DevSolar You're correct. Thanks. They're not const per se, but it's UB to write to them. — Bjorn A., Feb 09 '18 at 14:14

Stephan Lechner · Accepted Answer · 2018-02-09T12:54:29.573

With strcpy(vStrs[j], vStrs[i]), you copy the content of a string literal into another string literal. This is just as if you wrote strcpy("Max","Moritz"), yet string literals must not be modified (its undefined behaviour).

Anyway, the intent of your program is exchanging pointers to contents, not the contents per se. So if you change the program as follows, everything should be OK:

char *vStrs[] = {"Max", "Moritz", "Bolte", "Hans Huckebein", "Helene", "Antonius", "Boeck", "Maecke", "Lempel", "Schlich"};

int main()
{
    int num = sizeof(vStrs) / sizeof(vStrs[0]);

    for(int i=0; i<num-1; i++)
    {
        for(int j=i+1; j<num; j++)
        {
            if(strcmp(vStrs[j], vStrs[i]) < 0)
            {
                char *exchnge = vStrs[j];
                vStrs[j] = vStrs[i];
                vStrs[i] =exchnge;
            }
        }
    }

    for(int i=0; i<num; i++)
        printf("%s\n", vStrs[i]);

    return 0;
}

DevSolar · Answer 2 · 2018-02-09T13:19:32.503

...because you did not run your compiler in the strictest error-checking mode possible.

Some people believe that -Wall means "all warnings". It doesn't. Not even -Wall -Wextra even comes close to "strictest error-checking mode possible". I would start with -Wall -Wextra -pedantic -Wmissing-include-dirs -Wfloat-equal -Wundef -Wcast-align -Wwrite-strings -Wlogical-op -Wmissing-declarations -Wredundant-decls -Wshadow -Wno-system-headers -Wno-deprecated -Wunused-variable -Wunused-parameter -Wunused-function -Wunused, and then refer to the manual to find if there is more your compiler can do for you.

Adapt that last paragraph to your compiler of choice if you aren't using GCC. The compiler is your friend, give him every chance possible to keep you from making mistakes.

With those options enabled, you would get the following warning:

warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

Because this is what happens: "Max" is of type char * as far as the language standard is concerned, but -- as a string literal -- the memory pointed to is non-writeable. That is why your strcpy() segfaults on your machine. With -Wwrite-strings, you tell the compiler to instead make string literals char const * (like they are when using C++), enabling the compiler to generate a warning when you ignore the non-writeable nature of the literal.

Compiler warnings. Don't leave home without them.

C - Segmentation fault at strcpy()

2 Answers2