3

This code compiles fine but give segmentation fault error while running? Can anyone tell why?

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

int main() {
    const char s2[] = "asdfasdf";
    char* s1;

    strcpy(s1, s2);
    printf("%s", s1);

    return 0;
}
Amol Aggarwal
  • 2,674
  • 3
  • 24
  • 32
  • Code compiling fine does not really guarantee you a stable program. The compiler will do *some* checking for you, but there is no way to eliminate all possible unsafe programs, at least not in C. Many other languages are much more protective, but most of them don't let you access arbitrary memory the way C does. – quark Aug 18 '09 at 17:24

8 Answers8

13

You allocated space for a single pointer, s1, but not the bytes pointed at by s1.

A solution is to dynamically allocate memory for s1:

s1 = (char *)malloc(strlen(s2) + 1);
strcpy(s1, s2);

Keep in mind that you need to allocate one more byte of memory (the +1 in the call to malloc) than the number of characters in s2 because there is an implicit NULL byte at the end.

See C Memory Management (Stack Overflow) for more information.

Community
  • 1
  • 1
Andrew Keeton
  • 22,195
  • 6
  • 45
  • 72
  • 1
    Since s2 is a statically-allocated array, you can use sizeof() on it to get its size including the nul-terminator. strlen()+1 is correct for the general case, of course. – Tyler McHenry Aug 18 '09 at 16:53
  • 1
    Here are some more tips that I didn't want to clutter the answer with: 1) _Do_ check that `s1 != NULL` and 2) _Do_ call `free` on `s1` in any code that doesn't immediately return. – Andrew Keeton Aug 18 '09 at 16:58
  • Moreover, the pointer `s1` does not point anywhere determinate because it is not initialized. When it is initialized, it must point to enough space to store the string. The space might come from `malloc()`; it might come from elsewhere. – Jonathan Leffler Aug 18 '09 at 17:14
  • 1
    I need to add that C doesn't have namespaces. Totally irrelevant to the error in question, but a ANSI C compiler wouldn't even compile that code. – Leahn Novash Aug 18 '09 at 17:25
  • Leahn: Make that comment on the question. The error isn't in Andrew's answer, and it's possible the OP won't read your comment here. – quark Aug 18 '09 at 17:27
  • +1 Andrew Keeton, that's really scary to see that nobody check s1 != NULL – LB40 Aug 18 '09 at 17:32
4

You didn't allocate memory for s1. You have a pointer to s1 but no memory allocated for the strcpy to copy the value of s2 into.

char *s1 = malloc( strlen(s2) + 1 );

strcpy( s1, s2 );
nathan
  • 5,513
  • 4
  • 35
  • 47
3

You have not allocated any memory to s1. It is a pointer to nothing.

char* s1 = malloc(sizeof(s2));
strcpy(s1, s2);
printf("%s", s1);
free(s1);
Tyler McHenry
  • 74,820
  • 18
  • 121
  • 166
2

The problem is that s1 does not have any memory associated with it. strcpy does not call malloc().

You could either do:

char s1[10];

or

char *s1 = malloc(10);

Justin
  • 9,419
  • 7
  • 34
  • 41
2

What they all said, you need to allocate the space for s1. What everyone else has posted will work just fine, however, if you want a simpler way to allocate space for an existing string and copy it into a new pointer then use strdup like this:

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

using namespace std;

int main() {
    const char s2[] = "asdfasdf";
    char* s1;

    s1 = strdup(s2);
    printf("%s", s1);

    return 0;
}

Someone mentioned strdup earlier, that would be a way to use it. Most systems should support it since it is in the standard C libaries. But apparently some don't. So if it returns an error either write your own using the method already mentioned, or just use the method already mentioned ;)

Daniel Bingham
  • 12,414
  • 18
  • 67
  • 93
2

No one yet has pointed out the potential of strdup (String Duplicate) to address this problem.

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

using namespace std;

int main() {
    const char s2[] = "asdfasdf";
    char* s1;

    s1 = strdup(s2);  // Allocates memory, must be freed later.
    printf("%s", s1);

    free(s1);         // Allocated in strdup, 2 lines above
    return 0;
}
abelenky
  • 63,815
  • 23
  • 109
  • 159
1

You need to allocate the destination (and using namespace std; isn't C but C++, the rest of the code is C).

AProgrammer
  • 51,233
  • 8
  • 91
  • 143
0

You have to allocate memory to the pointer s1. If you don't do that, it will be pointing somewhere unknown, and thus arriving at the segmentation fault. The correct code should be:

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

int main() {
    const char s2[] = "asdfasdf";
    char* s1 = malloc(21 * sizeof(s2[0]));
    strcpy(s1,s2);
    printf("%s",s1);
    return 0;
}
Guilherme
  • 551
  • 4
  • 16
  • 1
    sizeof(char) is not necessary, since sizeof(char) == 1 is guaranteed by the standard. The (char*) cast is not necessary either since this is C, and void-pointers are implicitly convertible to any other kind of pointer. – Tyler McHenry Aug 18 '09 at 16:51
  • @Tyler McHenry sorry, I learned on school that it was necessary. Anyway, about the sizeof part, i guess it should be there in case of the author's change the code to other type. – Guilherme Aug 18 '09 at 16:56
  • 1
    Then just make it `sizeof(s2[0])` or `sizeof(*s2)` – Sean Bright Aug 18 '09 at 17:01