5

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;
}
S.S. Anne
  • 15,171
  • 8
  • 38
  • 76
Ram
  • 1,153
  • 4
  • 16
  • 34

3 Answers3

11

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.

Community
  • 1
  • 1
unwind
  • 391,730
  • 64
  • 469
  • 606
  • interesting point about structure assignment, except that I doubt any compiler could generate a padded structure copy that would be more efficient than the incredibly fast SIMD-style memory copies that modern CPUs support. – Alnitak Oct 30 '12 at 12:19
  • @Alnitak Well, the space of "any" is pretty large, and really *not* limited to "modern CPUs", which is exactly why I said *might*. – unwind Oct 31 '12 at 11:27
  • `*s = *student2` would copy also `*name`? and if `*name` is alloced, do i need to free `name` and `student2` ? – TomSawyer Jun 17 '20 at 07:32
  • It would copy `username`, i.e. the pointer value itself, not `*username` which is the first character of the name. No new allocation is magically done, you need to keep track of ownership and so on. – unwind Jun 17 '20 at 09:54
2

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.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
0

You set the username to "somebodyelse" after copying. And that changes only the local copy inside the function "changeme()". Try printing out student2 inside "changeme()" and you will see what I mean.

Prof. Falken
  • 24,226
  • 19
  • 100
  • 173