C - Copy a String without a library function-CodePudding

I want to create a function that takes a string literal and an array and copies the characters of string literal to the array. Could you please inform me what is the issue in the following code ? The output of this code is just a capital D with some whitespaces (" D") so i think a random location is accessed somehow.

#include <stdio.h>

int main(void)
{
    //Function Prototypes:
    void CopyAString(const char * s1, char s2[]);
    
    // Initialize the string literal str1 and an array s2 of size 12.
    const char *str1 = "Hello World";
    char s2[12];
    // In the function i pass the address of str1 and the array s2.
    CopyAString( &str1, s2 );

    for (int i = 0; i <= 12; i  ){
        printf("%c", s2[i]);
    }

}

void CopyAString(const char * s1, char s2[])
{
    const char * p1 = s1;
    int index = 0;

    while (*p1 != '\0') {
        s2[index] = *p1;
        index  ;
        p1  ;
    }
}

CodePudding user response：

Your program has two bugs:

First

Your function CopyAString does not write a '\0' character to the end of the string. This means that the string s2[] does not have a '\0' character and you would not be able to pass s2[] to functions like printf() or other functions that expect an "input" string to end with '\0'.

However, in your program, this is not a problem because the for loop expects a fixed-length string.

Second

In your program, the following problem is more important:

You pass &str as first argument to CopyAString instead of str.

This means that s1 (the first argument of CopyAString) does not point to the character 'H' of "Hello world" but it points to the first byte of the value stored in the variable str...

Note that the variable str is a pointer: It does not store a "value" (here: the string "Hello world") but it stores an address of a value!

If the string "Hello world" is stored in the RAM at address 0x20 44 00 40 (this means: 0x40004420 on an x86 or ARM computer or 0x20440040 on a PowerPC), the variable str will contain the value 0x20 44 00 40.

s1[0] will be 0x20 (which is the space character). s1[1] will be 0x44 (which is 'D')...

CodePudding user response：

For starters you should declare the function like

char * CopyAString( char *s1, const char *s2 );

similarly to the standard string function strcpy.

In this function call

CopyAString( &str1, s2 );

the expression &str1 has the type const char ** and yields the address of the pointer str1 but the function expects an argument expression of the type const char * that points to the first element of a string (the string literal).

Within the function you are not copying the terminating zero character

while (*p1 != '\0') {
    s2[index] = *p1;
    index  ;
    p1  ;
}

So the destination character array in general will not contain a string.

The function can be defined the following way

char * CopyAString( char *s1, const char *s2 )
{
    for ( char *p = s1; ( *p   = *s2   ); );

    return s1;
}

Within main instead of this for loop with the magic number 12 in the condition that invokes undefined behavior

for (int i = 0; i <= 12; i  ){
    printf("%c", s2[i]);
}

it is better to write

for ( char *p = s2; *p != '\0';   p ){
    printf("%c", *p );
} 
putchar( '\n' );