Custom strcpy is compiling fine, but not working properly

Question

I'm working on a C assignment that's basically making our own C String class. My partner and I are confident that we have the logic correct and our source files are compiling, but we're unable to get any output on a simple main file.

Our strcpy is as follows:

...
char *my_strcpy(char *s1, const char *s2)
{
    int r=0;    
    for (int i=0;s2[i]!='\0'; i++)
    {
        s1[i]=s2[i];
        r++;
    }
    s1[r+1]='\0';
    return s1;  
}
...

as for our main file we have:

...
const char *hello = "Hello World! ";
char pointer[1024];

my_strcpy(pointer, hello);
printf("%s\n", pointer);
...

any help would be greatly appreciated! thanks!

Answer 1

Revised code

char *my_strcpy(char *s1, const char *s2)
{
    int r=0;    
    for (int i=0;s2[i]!='\0'; i++)
    {
        s1[i]=s2[i];
        r++;
    }
    s1[r+1]='\0';
    return s1;  
}

Revised answer

You don't need both r and i. You have an off-by-one error at s1[r+1] = '\0';.

Working code

Correct for both original and revised versions of the question.

char *my_strcpy(char *s1, const char *s2)
{
    int i;    
    for (i = 0; s2[i] != '\0'; i++)
        s1[i] = s2[i];
    s1[i] = '\0';
    return s1; 
}

Original code

char *my_strcpy(char *s1, const char *s2)
{
    int r=0;    
    for (int i=0;*s2[i]!='\0'; i++)
    {
        *s1[i]=*s2[i];
        r++;
    }
    *s1[r+1]='\0';
    return *s1; 
}

Original answer

The code should not be compiling without lots of warnings (actually, they should be errors).

Throughout the code, the notation:

*s1[i]

is dereferencing a character as if it were a character pointer.

You're also 'off-by-one' in *s[r+1] = '\0';

You're returning a char as a char *.

You don't really need both i and r.

Answer 2

There are many wrong on your codes:

Compile Error:

1. my_strcpy need returning a pointer of char: char*, but you return a char.

Logic Error:

1. s2[i]!='\0' is right

So your code may look like this:

char *my_strcpy(char *s1, const char *s2)
{
    int r=0;    
    for (int i=0;s2[i]!='\0'; i++)
    {
        s1[i]=s2[i];
        r++;
    }
    s1[r]='\0';
    return s1; 
}

I think you are not familiar with the pointer in C.

Answer 3

You are returning *s1, it should be s1 since you need to return a pointer.

Answer 4

First, a coding error, you are defining int i = 0 in your for loop. All variables should be defined before you do anything else in the function.

Second, you want to copy over your last character which is the null at 'r'. Your loop increments r until you find a null character so why would you want to place the null character one place after where you find it? Change it to s1[r]='\0';

One thing that I have noticed with your my_strcpy function is that you are returning s1 but when you are calling my_strcpy, you are not assigning the return value to a pointer. This is fine because you first argument is an "output" because you are passing in a pointer. I would make the my_strcpy return void and get rid of the return s1; line all together.

All together, the my_strcpy would look like this

void my_strcpy(char *s1, const char *s2)
{
    int r = 0;
    int i = 0;
    for (i = 0; s2[i] != '\0'; i++)
    {
        s1[i] = s2[i];
        r++;
    }
    s1[r] = '\0';
}