simple string copy, but memcpy doesn't work

Question

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

void main()
{
    unsigned char str[] = "abcdefg";
    unsigned char *str1 = (unsigned char*)malloc(sizeof(str) -1);

    memcpy(str1, str, (sizeof(str)-1) );

    for(int i = 0; i<(sizeof(str1)); i++)
        printf("%c", str1[i]);

    free(str1);
}

I want copy the string str to str1 but the output is

abcd

It means that only pointer byte(4byte) is copied.

And i try

printf("%d", sizeof(str)-1 );

it's output is 7

What is my mistake?

@tkausl how can i use str1 space that i claimed malloc on memcpy? — sang oh, Sep 15 '19 at 08:58

score 1 · Accepted Answer · answered Sep 15 '19 at 09:01

it mean that only pointer byte(4byte) is copied.

No, it does not. You're assuming that your printout is correct, which it is not. You can use sizeof on arrays but not on pointers. Well, you can, but it means something different.

Everything is copied. You're just printing the first four characters. Change to:

for(int i = 0; i<(sizeof(str) - 1); i++)

Also, don't cast malloc. Here is why: Do I cast the result of malloc?

Mihir Luthra · Answer 2 · 2019-09-15T09:09:12.143

str1 is a pointer whereas str is a character array. When you say sizeof(str1) in your for loop, it iterates 4 times because sizeof(str1) must have evaluated to 4(a 32 bit compiler) whereas sizeof(str) evaluates to the correct length.

You should read What is the size of a pointer? once to know more about size of pointers.

Fixed code:

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

void main()
{
    unsigned char str[] = "abcdefg";
    unsigned char *str1 = (unsigned char*)malloc(sizeof(str) -1);

    memcpy(str1, str, (sizeof(str)-1) );

    for(int i = 0; i<(sizeof(str) - 1); i++)
        printf("%c", str1[i]);

    free(str1);
}

score 1 · Answer 3 · answered Sep 15 '19 at 09:07

First the mistakes and then the correct code: Mistakes:

Don't ever use sizeof(str) to get the length of the string. It doesn't work with pointers. Instead, use strlen(str) + 1.
You are subtracting 1 from the size of the string on malloc call. Why? You are not making space for the ending NULL character.
When copying strings, if you know that the destination string is large enough to store the source, use strcpy instead of memcpy. If you only want an additional size parameter like in memcpy, use strncpy. memcpy is not meant to deal with strings, but with plain arrays.
The correct type to use for strings is char, not unsigned char.
Not really a mistake, but to print a string you can use printf("%s", str) or puts(str). Why bother with a for loop?
void main() is prohibited by the C standard.

Correct code:

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

int main()
{
    char str[] = "abcdefg";
    char str1* = malloc(strlen(str) + 1);
    strcpy(str1, str);
    puts(str1);
    free(str1);
    //return 0; - default in C99 and later
}

@n.m. still bad practice. Btw he did `sizeof(str1)` in his question. Look at his for loop. — DarkAtom, Sep 15 '19 at 09:09
No mistake made by OP justifies saying that str is a pointer. `sizeof(str)` returns tge correctvsize of str array. Whether it is a good practice or not is a different question. You cannot discuss good and bad practices based on distorted facts. — n. m. could be an AI, Sep 15 '19 at 09:15
@n.m. Please use some punctuation marks, man. I cannot understand anything of what you said. — DarkAtom, Sep 15 '19 at 09:18

simple string copy, but memcpy doesn't work

3 Answers3