string copy() function return misunderstood? (C)

Question

(EDIT)

Probable misunderstanding in updating and returning a string from a function in C.

I'm a student and have just started trying out dynamic memory allocation techniques. Recently, I was told to work on a string copy() function similar to the strcpy() function in C under <string.h>.

It is supposed to be taking two arguments: char c1[],char c2[]. Momentarily, I am focusing on a constraint wherein the length of string c1 is lesser than the string c2. This is the case, we will be copying a larger string into a smaller one. So, naturally it would have to expand the former string's size. (I have defined a function len() which returns the length of the string. It's working fine)

Code:

char* copy(char c1[],char c2[])
{
    int i=0;
    int lc1=len(c1);
    int lc2=len(c2);
    if(lc1<lc2)
    {
                //are the following two lines allowed?
        c1=(char*) malloc(lc1*sizeof(char));
        c1=(char*) realloc(c1,lc2*sizeof(char));
        for(i=0;i<lc2;i++)
        {
            *(c1+i)=c2[i];
        }
        *(c1+lc2)='\0';
    }
    return c1;
}

*I won't be surprised if this code has multiple issues! * I tried testing it as follows:

#include <stdio.h>
#include <string.h>
#include "mystring.h"
main()
{
    int i;
    char s1[12],s2[12];
    gets(s1);
    gets(s2);
    printf("copy: %s",copy(s1,s2));
    printf("\ns1: %s",s1);
}

I got: output

I wanted both of the outputs to be the same, i.e. copy's and s1's. It happens to be that only this functions only when I print it along with the invokation. I was expecting it to print the copy when I printed s1.

Where have I gone wrong? I think it's something related to the scope of the variables I'm dealing with, or the pointers and memory allocation as a whole!

EDIT: I had made an alternative-function xcopy:

char* xcopy(char c1[])
{
    int i=0;
    int lc1=len(c1);
    char* c2=(char*) malloc((lc1+1)*sizeof(char));
    for(i=0;i<lc1;i++)
    {
        *(c2+i)=c1[i];
    }
    *(c2+lc1)='\0';
    return c2;
}

I realised this function was not solving the actual problem.

---

Thankyou for your help!!

Answer 1

The function copy does not make sense.

For starters it is not necessary that the array pointed to by the parameter c1 contains a string. So this line

int lc1=len(c1);

invokes undefined behavior.

Even if the array pointed to by the pointer c1 contains a string the stored string can be much lesser than the size of the array. So again this line

int lc1=len(c1);

with the following if statement

if(lc1<lc2)

does not make sense.

Also allocating memory two times

c1=(char*) malloc(lc1*sizeof(char));
c1=(char*) realloc(c1,lc2*sizeof(char));

is unsafe, redundant and inefficient.

Actually the function does not copy the string pointed to by the pointer c2 in the destination array. Thus using the parameter c1 again does not make sense. Your function looks like POSIX function strdup if to remove the first senseless parameter.

You did not show how the function len that calculates length of a string but it seems it does not count the terminating zero character of a string. In this case you need at least to allocate memory using the expression ( lc2 + 1 ) * sizeof( char ) to reserve memory for the terminating zero character '\0'. Otherwise this statement

*(c1+lc2)='\0';

invokes undefined behavior.

It is the responsibility of the user of the function to provide a destination array large enough to store the source string. The function shall not allocate any memory.

By analogy with the standard C function strcpy the function can look the following way

char * copy( char s1[], const char s2[] )
{
    for ( char *p = s1; ( *p++ = *s2++ ) != '\0'; );

    return s1;
}

Pay attention to that the function gets is unsafe and is not supported by the C Standard. Instead use standard C function fgets. It can append a string with the new line character '\n'.

To remove it you can write for example

c2[ strcspn( c2, "\n" ) ] = '\0';