I made my own strcpy function, but it is not working. How to fix it?

Question

I attempted to make my own mystrcpy() function, which takes the same arguments as the standard function. It is not responding. The array does not get copied.

size_t Mystrlen(const char* s)
{
    int i = 0;
    while (s[i] != '\0')
    {
        i++;
    }
    return i;
}

char* Mystrcpy(char* s1, const char* s2)
{
    for (int i = 0; i < Mystrlen(s2); i++)
        s1[i] = s2[i];
    return s1;
}

int main()
{
    char s1[50];
    char s2[50];
    cout << "enter the value of second string\n";
    cin >> s2;
    Mystrcpy(s1, s2);
}

https://godbolt.org/z/zWxqxn3Kx

Answer 1

1. You'd better call Mystrlen once before the loop, instead of each iteration (to check the loop condition). It's very inefficient this way.
2. You didn't copy the zero termination to the destination string.
3. Not related to your bug, but anyway it is better to avoid using namespace std - see here Why is "using namespace std;" considered bad practice?
4. As @Pete Becker commented, MyStrcpy doesn't need to call Mystrlen at all - see above. I kept it the way it was assuming it is done like this for learning purposes.
5. In real production code, the length of the output char array (s1) must be validated, otherwise Mystrcpy can cause a buffer overrun (if s1 is allocated number of chars less than required for the content of s2).

Fixed code (up to the notes above):

#include <iostream>

// size_t Mystrlen(const char* s) ...   // Same as in your code

char* Mystrcpy(char* s1, const char* s2)
{
    size_t len = Mystrlen(s2);  // Call once before the loop
    for (int i = 0; i < len; i++)
    {
        s1[i] = s2[i];
    }
    s1[len] = '\0';     // Add zero termination
    return s1;
}

int main()
{
    char s1[50];
    char s2[50];
    std::cout << "enter the value of second string\n";
    std::cin >> s2;
    Mystrcpy(s1, s2);
    std::cout << "copied string: " << s1 << std::endl;
}