2

For a homework assignment I'm supposed to implement all 22 functions of the string.h library (2fun2handle). I've gotten a lot of functions down, but am running into a bit of trouble when trying to implement strcpy.

After quite a few revisions, here is my attempt at the function:

char *new_strcpy(char *dest, const char *src){
    char *copy = (char *) src;
    for(; *copy != '\0'; ++copy, ++dest)
        *dest = *copy;
    return dest;
}

I thought this would work fine. However, when testing my code by doing a simple example like this:

char src[50], dest[50];
new_strcpy(src,  "src");
new_strcpy(dest,  "dest");
printf("dest: %s\n", dest);
printf("src: %s\n", src);

My output ends up looking like this:

dest: dest$
src: src$$$$

When it should just look like this:

dest: dest
src: src

Before implementing this function, I've copied src strings to dest strings by using pointers without issue.. so I'm not sure why this is happening now. Is there something obvious that I'm doing wrong? Also note that I've tried doing this with a while loop and looping until *copy is empty, and I've tried directly looping through the original *dest and *src arguments that get passed in.

Alex
  • 2,145
  • 6
  • 36
  • 72
  • 1
    You're not null-terminating the destination - the for loop breaks and you never copy the null. – Katie Jan 20 '15 at 20:20
  • You must null-terminate your string, either by doing this explicitly after the loop or by using `do ... while`. `strcpy` returns (the initial value of) `dest`. You return the pointer to the end of the `dest` string (which might even be a more useful return value). – M Oehm Jan 20 '15 at 20:22
  • Not really answering your question, but this method of strcpy is really cool in my view: while(*dest++=*src++); – Atuos Jan 20 '15 at 20:24

1 Answers1

5

You never mark the actual string as finished, using the special zero character:

char *new_strcpy(char *dest, const char *src){
    char *copy = (char *) src;
    for(; *copy != '\0'; ++copy, ++dest)
        *dest = *copy;
    *dest=0;
    return dest;
}

Note that you don't need the copy variable, it's just garbage the compiler will remove for you.

Edit: as requested, the classic strcpy function that doesn't cast away const and is less verbose is as follows:

char *new_strcpy(char *dest, const char *src)
{
  char *ret = dest;            // needed to return, useless as this is
  while(*dest++ = *src++);     // straight byte copy, very unoptimized
  //*dest=0;                   // no longer needed since the copy happens before the check now
  return ret;                  // and return the original buffer (note that you were returning the end)
}
Blindy
  • 65,249
  • 10
  • 91
  • 131
  • 1
    Casting away the `const` qualifier is very bad practice. Since you are correct that the `copy` variable is unnecessary, I suggest showing how to write the function without it. – user3386109 Jan 20 '15 at 20:22
  • Does `*dest = 0` actually act as a terminating character or should I be using `'\0'` – Alex Jan 20 '15 at 20:23
  • @Alex `'\0'` is the character representation equivalent to `0` – clcto Jan 20 '15 at 20:24
  • @Alex There are some characters that have special meaning, the '\0' or "zero" character is not printable like a letter or a digit, and it's used to mark the finish of a `string`. – umlcat Jan 20 '15 at 20:40
  • I assume he knows that since his initial solution checks for 0 endings. It was actually pretty damn close, and a quick peek under a debugger would have certainly tipped him off to the final solution. – Blindy Jan 20 '15 at 20:43
  • 1
    The `while` loop terminated _because_ the terminating 0 has been copied. Hence `*dest = 0` is redundant (and in fact harmful, since it modifies one character more than necessary). – user58697 Jan 20 '15 at 23:54
  • @user58697 is right; even worse: this implementation 0-terminates twice and potentially overflows the buffer (copying the string "dest" requires a 6 byte destination buffer). – mafso Jan 21 '15 at 10:24