So I have this header given to me

int mystrcpy(char *c, const char* s);

And I have to implement strcpy function myself. But when I do it and run the program, the console stops working, which I believe is a sign of memory address violation.

I tried this:

int mystrcpy(char *c, const char* s){int i=0;for(i=0;i<strlen(s);i++)c[i]=s[i];c[++i]=NULL;cout<<c;return 0;}

Full code:

#include<iostream>#include<cstring>using namespace std;class Persoana{char *nume;int an;float inaltime;public:int my_strcpy(char *c, const char*s);};int Persoana::my_strcpy(char *c, const char* s){//code I need to insertreturn 0;}int main(){Persoana a;cout << endl;cout<<a.my_strcpy("maria","george");return 0;}
3

Best Answer


Other posters have posted implementations of strcpy - Why you are using this in C++ code? That is an interesting question as C++ very rarely uses C style strings

The other problem is with its usage:

int main(){Persoana a;cout << endl;cout<<a.my_strcpy("maria","george");return 0;}

The strings "maria" and "george" are read only. Instead create an empty read/ write string as shown below that is long enough - i.e. 7 characters (do not forget the null character)

So the code should be

int main(){Persoana a;char copy_of_string[7];cout << endl;cout<<a.my_strcpy(copy_of_string,"george");return 0;}

Your loop itself is OK (but inefficient, since you call strlen() on every iteration). The real problem is when you go to insert the null terminator at the end of the copied string. You are incrementing i again before inserting the terminator. DON'T do that. i is already at the correct index after the loop ends, so just use it as-is:

int mystrcpy(char *c, const char* s);{int i, len = strlen(s);for(i = 0; i < len; ++i)c[i] = s[i];c[i] = '\0'; // <-- NO ++ HERE!cout << c;return 0;}

That being said, the simplest way to implement strcpy without using i at all is as follows:

int mystrcpy(char *c, const char* s){char *p = c;while (*p++ = *s++);cout << c;return 0;}

If you remove the cout then it becomes simpler:

int mystrcpy(char *c, const char* s){while (*c++ = *s++);return 0;}

No matter what, make sure that when you call mystrcpy(), c is pointing to a char[] buffer that is allocated to at least strlen(s)+1 chars in size, otherwise the code will have undefined behavior.

The for loop

for(i=0; i < strlen(s); i++)

aborts when i < strlen(s) becomes false, which is the case when i is equal to strlen(s). So that will be the value of i when the loop ends.

C strings are NULL-terminated as you know, so you need strlen(s) + 1 bytes reserved for c. Since you increment i again before writing the '\0' character, you're using strlen(s) + 2 bytes starting at c.

If c is exactly the size that's needed (strlen(s) + 1), that may lead to an access violation, since you're writing past the end of the allocated memory.

So instead of

c[++i]=NULL;

write

c[i]='\0';

Hope this makes sense!