How to properly implement strcpy in c?

Question

According to this: strcpy vs strdup, strcpy could be implemented with a loop, they used this while(*ptr2++ = *ptr1++). I have tried to do similar:

#include <stdio.h>
#include <stdlib.h>
int main(){
    char *des = malloc(10);
    for(char *src="abcdef\0";(*des++ = *src++););
    printf("%s\n",des);
}

But that prints nothing, and no error. What went wrong?

Thanks a lot for answers, I have played a bit, and decided how best to design the loop to see how the copying is proceeding byte by byte. This seems the best:

#include <stdio.h>
#include <stdlib.h>

int main(){
    char *des = malloc(7);
    for(char *src="abcdef", *p=des; (*p++=*src++); printf("%s\n",des));
}

Answer 1

You are incrementing des so naturally at the end of the cycle it will be pointing past the end of the string, printing it amounts to undefined behavior, you have to bring it back to the beginning of des.

#include <stdio.h>
#include <stdlib.h>

int main(){
    int count = 0;
    char *des = malloc(10);

    if(des == NULL){
       return EXIT_FAILURE; //or otherwise handle the error
    }

    // '\0' is already added by the compiler so you don't need to do it yourself
    for(char *src="abcdef";(*des++ = *src++);){
        count++; //count the number of increments
    }
    des -= count + 1; //bring it back to the beginning
    printf("%s\n",des);
    free(dest); //to free the allocated memory when you're done with it
    return EXIT_SUCCESS;
}

Or make a pointer to the beginning of des and print that instead.

#include <stdio.h>
#include <stdlib.h>

int main(){
 
    char *des = malloc(10);

    if(des == NULL){
       return EXIT_FAILURE; //or otherwise handle the error
    }

    char *ptr = des;
    for(char *src="abcdef";(*des++ = *src++);){} //using {} instead of ;, it's clearer

    printf("%s\n",ptr);
    free(ptr) // or free(dest); to free the allocated memory when you're done with it
    return EXIT_SUCCESS;

}

Answer 2

In this loop

for(char *src="abcdef\0";(*des++ = *src++););

the destination pointer des is being changed. So after the loop it does not point to the beginning of the copied string.

Pay attention to that the explicit terminating zero character '\0' is redundant in the string literal.

The loop can look the following way

for ( char *src = "abcdef", *p = des; (*p++ = *src++););

And then after the loop

puts( des );

and

free( des );

You could write a separate function similar to strcpy the following way

char * my_strcpy( char *des, const char *src )
{
    for ( char *p = des; ( *p++ = *src++ ); );

    return des;
}

And call it like

puts( my_strcpy( des, "abcdef" ) )'
free( des );

Answer 3

printf("%s\n",des); is undefined behavior (UB) as it attempts to print starting beyond the end of the string written to allocated memory.

Copy the string

Save the original pointer, check it and free when done.

const char *src = "abcdef\0"; // string literal here has 2 ending `\0`, 
char *dest = malloc(strlen(src) + 1);  // 7

char *d = dest;
while (*d++ = *src++);
printf("%s\n", dest);
free(dest);

Copy the string literal

const char src[] = "abcdef\0"; // string literal here has 2 ending `\0`, 
char *dest = malloc(sizeof src);  // 8

for (size_t i = 0; i<sizeof src; i++) {
  dest[i] = src[i];
}

printf("%s\n", dest);
free(dest);

Answer 4

You allocate the destination buffer des and correctly copy the source string into place. But since you are incrementing des for each character you copy, you have moved des from the start of the string to the end. When you go to print the result, you are printing the last byte which is the nil termination, which is empty.

Instead, you need to keep a pointer to the start of the string, as well as having a pointer to each character you copy.

The smallest change from your original source is:

#include <stdio.h>
#include <stdlib.h>
int main(){
    char *des = malloc(10);
    char *p = des;
    for(char *src="abcdef";(*p++ = *src++););
    printf("%s\n",des);
}

So p is the pointer to the next destination character, and moves along the string. But the final string that you print is des, from the start of the allocation.

Of course, you should also allocate strlen(src)+1 worth of bytes for des. And it is not necessary to null-terminate a string literal, since that will be done for you by the compiler.

Answer 5

You just need to remember the original allocated pointer.

Do not program in main. Use functions.

#include <stdio.h>
#include <stdlib.h>

size_t strSpaceNeedeed(const char *str)
{
    const char *wrk = str;
    while(*wrk++);
    return wrk - str;
}

char *mystrdup(const char *str)
{
    char *wrk;
    char *dest = malloc(strSpaceNeedeed(str));

    if(dest)
    {
        for(wrk = dest; *wrk++ = *str++;);
    }   
    return dest;
}

int main(){
    printf("%s\n", mystrdup("asdfgfd"));
}

or even better

size_t strSpaceNeedeed(const char *str)
{
    const char *wrk = str;
    while(*wrk++);
    return wrk - str;
}

char *mystrcpy(char *dest, const char *src)
{
    char *wrk = dest;
    while((*wrk++ = *src++)) ;
    return dest;
}

char *mystrdup(const char *str)
{
    char *wrk;
    char *dest = malloc(strSpaceNeedeed(str));

    if(dest)
    {
        mystrcpy(dest, str);
    }   
    return dest;
}

int main(){
    printf("%s\n", mystrdup("asdfgfd"));
}

Answer 6

But that prints nothing, and no error. What went wrong?

des does not point to the start of the string anymore after doing (*des++ = *src++). In fact, des is pointing to one element past the NUL character, which terminates the string, thereafter.

Thus, if you want to print the string by using printf("%s\n",des) it invokes undefined behavior.

You need to store the address value of the "start" pointer (pointing at the first char object of the allocated memory chunk) into a temporary "holder" pointer. There are various ways possible.

#include <stdio.h>
#include <stdlib.h>

int main (void) {
    char *des = malloc(sizeof(char) * 10);
    if (!des)
    {
        fputs("Error at allocation!", stderr);
        return 1;
    }

    char *tmp = des;

    for (const char *src = "abcdef"; (*des++ = *src++) ; );
    des = temp;

    printf("%s\n",des);

    free(des);
}

Alternatives:

#include <stdio.h>
#include <stdlib.h>

int main (void) {
    char *des = malloc(sizeof(char) * 10);
    if (!des)
    {
        fputs("Error at allocation!", stderr);
        return 1;
    }

    char *tmp = des;

    for (const char *src = "abcdef"; (*des++ = *src++) ; );

    printf("%s\n", tmp);

    free(tmp);
}

or

#include <stdio.h>
#include <stdlib.h>

int main (void) {
    char *des = malloc(sizeof(char) * 10);
    if (!des)
    {
        fputs("Error at allocation!", stderr);
        return 1;
    }

    char *tmp = des;

    for (const char *src = "abcdef"; (*tmp++ = *src++) ; );

    printf("%s\n", des);

    free(des);
}

---

Side notes:

• "abcdef\0" - The explicit \0 is not needed. It is appended automatically during translation. Use "abcdef".

• Always check the return of memory-management function if the allocation succeeded by checking the returned for a null pointer.

• Qualify pointers to string literal by const to avoid unintentional write attempts.

• Use sizeof(char) * 10 instead of plain 10 in the call the malloc. This ensures the write size if the type changes.

• int main (void) instead of int main (void). The first one is standard-compliant, the second not.

• Always free() dynamically allocated memory, since you no longer need the allocated memory. In the example above it would be redundant, but if your program becomes larger and the example is part-focused you should free() the unneeded memory immediately.