0

I tried to make strcpy myself. It should work, I even copied and pasted the (almost exact code) from someones post here about strcpy. Both give me a "Segmentation Fault".

char* strcpy(char * destination, const char * source)
{
    while( (*destination++ = *source++) != '\0' ) 
        ;
    return destination;
}

What's wrong with this code? 

char* a = "hello";
cout << strcpy(a, "Haha") << endl;
Normal People Scare Me
  • 839
  • 4
  • 12
  • 27
  • 5
    Show the code where you call this function. – Carl Norum May 21 '13 at 18:31
  • 16
    My bet is that this being called w/ string literal as destination – Shafik Yaghmour May 21 '13 at 18:32
  • Why couldn't C++03 make `char *` string literals banned as well :( – chris May 21 '13 at 18:33
  • @ShafikYaghmour, that was my guess too. And OP's edit confirms. – Carl Norum May 21 '13 at 18:33
  • 1
    The question is why you're even making or using `strcpy` in the first place. This is C++, plus there's already a perfectly fine one in the `cstring` header. – chris May 21 '13 at 18:33
  • 1
    possible duplicate of [Why are string literals const?](http://stackoverflow.com/questions/14570993/why-are-string-literals-const) – Carl Norum May 21 '13 at 18:35
  • 4
    @chris problably because i wanna learn and don't belive in magic – Normal People Scare Me May 21 '13 at 18:35
  • 3
    When you return from strcpy `destination` points to the end of the string so even if you get rid of the segfault it won't print anything out – Captain Obvlious May 21 '13 at 18:35
  • 1
    @Normal I don't believe in magic either, but somehow I can reconcile that with allowing myself to use standard headers... – Nicholas Wilson May 21 '13 at 18:36
  • There's nothing magic about copying bytes from one place to another. – Brian Roach May 21 '13 at 18:37
  • @NormalPeopleScareMe, It seems like a kind of pointless exercise, to be honest. Learning about C strings is fine and dandy, but best done with a [structured book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). The exercises in them are meant to teach. – chris May 21 '13 at 18:37
  • 2
    for all you people saying this is pointless, he has clearly learned something from this exercise. relax. I agree about the book though – im so confused May 21 '13 at 18:38
  • 1
    @NormalPeopleScareMe It's great that you're trying things out, and would be better if you started with a good book. But one other thing you need to do is start paying attention to compiler warnings, and compiling your code with all warnings turned on. `char* a = "hello";` should cause a warning about a deprecated conversion to be emitted. That should've been a clue that maybe you're trying to do something illegal. – Praetorian May 21 '13 at 18:45

4 Answers4

6

You're trying to overwrite a string literal. That causes undefined behaviour. Declare a as an array instead:

char a[] = "hello";

Your strcpy implementation has a bug, too (assuming the normal semantics). It should return a pointer to the beginning of the destination buffer, not the end.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469
4

You are trying to write to data segment since "hello" is stored there.

Therefore, when you call strcpy you get segmentation fault.

Try:

char a[] = "hello";
cout << strcpy(a, "Haha") << endl;

instead.

EDIT: Inside your strcpy function, after the copy, destination will point to end of the string, you need to return beginning of the string instead.

taocp
  • 23,276
  • 10
  • 49
  • 62
3

a is a pointer to a string literal:

char* a = "hello";

Trying to modify a string literal is undefined behavior. As Carl suggested, if you initialize an array with the string literal instead it will work:

char a[] = "hello" ;
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
3

Besides everything mentioned above and below about string literals, you're also returning a pointer to the END of your string, so even if you avoid the segmentation fault, you'll print "".

i Code 4 Food
  • 2,144
  • 1
  • 15
  • 21