char array copies whole word instead of single letter on last iteration

88 views Asked by At
#include <iostream>
using namespace std;
void myStrcpy(char [], char []);  
int main(int argc, const char * argv[]) {
    char c1[] = "htako";
    char c2[] = "Mark";

    myStrcpy(c1, c2);
    cout << c2 << endl;

    return 0;
}

void myStrcpy (char c1[], char c2[]) {
    int i = 0;
    while(c1[i] != '\0') {
        c2[i] = c1[i];
        cout << c1[i] << "  ::: " << i << endl;
        i++;
    }
}

When I step through the debugger everything works fine until i = 4, on the 4th interation it goes from (3rd iteration) htak to htakohtako. I do not understand why it copies the whole word over on the last iteration. When I print out c1[i] on the fourth iteration it prints out an o as expected but where does this extra htako come from at the end?

2

There are 2 answers

0
ash108 On

Two problems with your code:

  • you are using c-based strings while forgetting to write terminating null char to c2
  • you don't have enough space in c2 for that null char.

To fix the first problem, add c2[i] = 0; after your while loop.

To fix the second one, use char c2[6] = "Mark"; Note that you need 6 here to hold 5 chars from "htako" + terminating null char.

In more detail: char c1[] = "htako"; actually defines char array of length 6:

{ 'h', 't', 'a', 'k', 'o', \0 }.

char c2[] = "Mark"; defines shorter char array of length 5:

{ 'M', 'a', 'r', 'k', \0 }

So when your while loop ends after coping c1 to c2, you have in the place of c2:

{ 'h', 't', 'a', 'k', 'o' }

Note that you don't have terminating null char. Also, cince c2 is implicitly declared to have length of 5, you don't have space to add it. So when you outputting the modified c2 to cout, since you are using char[], cout actually receives char * without length and, by C-strings convention, determines the length of the string by searching for terminating null char. Since you don't have that null char, cout will go past the end of c2 searching for it and accessing data outside valid ares is an undefined behaviour for C++ program. On your particular machine, stack grows up so c1 happens to be right after c2 and you see "htakohtako" printed. But really your program could also fail with SEGV or do any number of unplesant things. This type of error is called buffer overflow and is the root of many of security vulnerabilities. Avoid that at any cost.


But really, std::string is more suitable and safe option here, unless you are just playing around with how C strings work internally.

1
Eyal Cinamon On

The answer to your question has to do with how the memory is laid out. If you inspect the memory you would see:

c2              c1 
M, a, r, k, \0, h, t, a, k, o, \0

There are 2 problems in your code.

  1. You have a buffer overflow c2 is only 5 bytes (Mark + null terminator), but you are overwriting the null terminator since c1 is 6 bytes long (htako + null terminator)
  2. You are not null terminating your string

You are seeing the string appearing to grow because you overwrite the null terminator. In other situations your program may crash.