Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

memcpy of structure having pointers as members in C

I have a structure with some pointers as members and I am trying to do memcpy and I have been suggested that I should not use memcpy in this case as memcpy do a shallow copy (meaning it copies pointers) rather deep copy (meaning copying what pointers point to).

But I am not sure why it is not making any difference in the following program: Please have a look at code and output and please explain why it is not a shallow copy in this case?

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

struct student {
    char *username;
    char *id;
    int roll;
};

void print_struct(struct student *);
void print_struct_addr(struct student *);
void changeme(struct student *);

int main (void) {
    struct student *student1;
    char *name = "ram";
    char *id = "200ABCD";
    int roll = 34;

    student1 = (struct student *)malloc(sizeof(struct student));
    student1->username = name; 
    student1->id = id;
    student1->roll = roll; 
    print_struct_addr(student1);
    print_struct(student1);
    changeme(student1);
    print_struct(student1);
    print_struct_addr(student1);
    return 0;
}

void print_struct(struct student *s) {
    printf("Name: %s\n", s->username);
    printf("Id: %s\n", s->id); 
    printf("R.No: %d\n", s->roll);
    return; 
}

void print_struct_addr(struct student *s) {
    printf("Addr(Name): %x\n", &s->username);
    printf("Addr(Id): %x\n", &s->id);
    printf("Addr(R.No): %x\n", &s->roll);
    return;
}

void changeme(struct student *s) {
    struct student *student2;
    student2->username = "someone";
    student2->id = "200EFGH";
    student2->roll = 35;
    print_struct_addr(student2);
    memcpy(s, student2, sizeof(struct student));
    student2->username = "somebodyelse";
    return;
}

output:

Addr(Name): 9b72008
Addr(Id): 9b7200c
Addr(R.No): 9b72010
Name: ram
Id: 200ABCD
R.No: 34
Addr(Name): fa163c
Addr(Id): fa1640
Addr(R.No): fa1644
Name: someone
Id: 200EFGH
R.No: 35
Addr(Name): 9b72008
Addr(Id): 9b7200c
Addr(R.No): 9b72010

If memcpy does a shallow copy, how come student1->username is NOT "somebodyelse".

Please explain in which scenario, this code can create problem, I want student2 information in student1 after changeme() call in main() and should be able to use this modified student1 data afterwards.

I have been suggested to NOT to use memcpy() here, but it seems to be working fine.

Thanks

This is the modified code: But still I dont see concept of shallow copy here:

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

struct student {
    char *username;
    char *id;
    int roll;
};

void print_struct(struct student *);
void print_struct_addr(struct student *);
void changeme(struct student *);

int main (void) {
    struct student *student1;
    char *name = "ram";
    char *id = "200ABCD";
    int roll = 34;

    student1 = malloc(sizeof(*student1));
    student1->username = name; 
    student1->id = id;
    student1->roll = roll; 
    print_struct_addr(student1);
    print_struct(student1);
    changeme(student1);
    print_struct(student1);
    print_struct_addr(student1);
    return 0;
}

void print_struct(struct student *s) {
    printf("Name: %s\n", s->username);
    printf("Id: %s\n", s->id); 
    printf("R.No: %d\n", s->roll);
    return; 
}

void print_struct_addr(struct student *s) {
    printf("Addr(Name): %x\n", &s->username);
    printf("Addr(Id): %x\n", &s->id);
    printf("Addr(R.No): %x\n", &s->roll);
    return;
}

void changeme(struct student *s) {
    struct student *student2;
    student2 = malloc(sizeof(*s));
    student2->username = strdup("someone");
    student2->id = strdup("200EFGH");
    student2->roll = 35;
    print_struct_addr(student2);
    memcpy(s, student2, sizeof(struct student));
    student2->username = strdup("somebodyelse");
    free(student2);
    return;
}
like image 931
Ram Avatar asked Oct 30 '12 12:10

Ram


Video Answer


2 Answers

This:

struct student *student2;
student2->username = "someone";
student2->id = "200EFGH";
student2->roll = 35;

Is writing into non-allocated memory, invoking undefined behavior. You need to make sure student2 is pointing at somewhere valid, before writing.

Either allocate it, or use an on-stack instance since you're just going to copy from it anyway.

Of course, this entire business of initializing student2 and then overwriting s with it is needlessly complicated, you should just modify s directly.

Also, this:

student1 = (struct student *)malloc(sizeof(struct student));

is better written, in C, as:

student1 = malloc(sizeof *student1);

This removes the pointless (and potentially dangerous) cast, and makes sure the size is the proper one for the type, replacing a dependency checked by the programmer with one handled by the compiler.

Thirdly, it's a bit of a classic "symptom" of the beginning C programmer to not realize that you can assign structures. So, instead of

memcpy(s, student2, sizeof *s);

You can just write:

*s = *student2;

And have the compiler to the right thing. This might be a performance win, since the structure can contain a lot of padding which the assignment can be aware of and not copy, but which memcpy() cannot ignore.

like image 128
unwind Avatar answered Oct 19 '22 13:10

unwind


That it works at all is a fluke. In your changeme() function you are creating a new pointer for student2, but you are not allocating the memory for it.

Secondly, in that same function you are changing student2 after you've copied it into s. A shallow copy does not mean that any pointers within the copies are shared - it means that the values of the pointers themselves are also copied. So when you change student2->username after the memcpy it doesn't change the value of s->username.

As you progress, you also need to be more careful with the allocation of memory within those structures. AFAICR, if you use a constant literal string then the pointer will point at a chunk of statically initialised data within the program's memory space. However a more rigourous design would malloc() and free() dynamic memory for those elements. If you ever needed a statically initialised value you would use strdup() or similar to copy the string from the static space into heap memory.

like image 20
Alnitak Avatar answered Oct 19 '22 12:10

Alnitak