views:

236

answers:

7

Hello,

I have a pointer to a structure and I need to implement a method that will copy all of the memory contents of a structure. Generally speaking I need to perform a deep copy of a structure.

Here's the structure:

typedef struct { 
    Size2f spriteSize;

    Vertex2f *vertices;

    GLubyte *vertex_indices;
} tSprite;

And here's the method I've implemented that should copy the structure:

tSprite* copySprite(const tSprite *copyFromMe)
{

    tSprite *pSpriteToReturn = (tSprite*)malloc( sizeof(*copyFromMe) );

    memcpy(pSpriteToReturn, copyFromMe, sizeof(*copyFromMe) );

    return pSpriteToReturn;
}

The problem is that I'm not sure that arrays "vertices" and "vertex_indices" are going to be copied properly. What is going to be copied in this way? Address of the array or the array itself?

Should I copy the arrays after copying the structure? Or is it enough just to copy the structure?

Something like this:

...
pSpriteToReturn->vertices = (Vector2f*)malloc( sizeof(arraysize) );
memcpy(pSpriteToReturn->vertices, copyFromMe->vertices, sizeof(arraysize) );
...

Thank you in advance.

+3  A: 

This partially depends on your requirements. If you don't copy the arrays, both structures will be pointing to the same array, which may or may not be a problem.

Brian Young
+3  A: 

Your scheme is going to copy the addresses of the arrays. The "copy" tSprite returned is going to have pointers to the same data (in memory) as the passed in one.

If you want a true deep-copy, you'll need to copy the arrays (and any members of their elements) manually.

Kevin Montrose
+4  A: 

As a rule of thumb, don’t ever use memcpy in C++ in normal code (it might crop up in very low-level code, e.g. in allocators)1). Instead, create a suitable copy constructor and overload operator = (the assignment operator) to match it (and a destructor – rule of three: “if you implement either of copy constructor, operator = and destructor, you must implement all three).

If you do not implement your own versions of the copy constructor an the assignment operator, C++ will create default versions for you. These versions will implement a shallow copy (much like what memcpy would do), i.e. in your case the array contents would not be copied – only the pointers.


1) Incidentally, the same goes for malloc and free. Don’t use them, instead use new/new[] and delete/delete[].

Konrad Rudolph
Thanks, but what about copying c-styled arrays? How can I copy them without using memcpy?
Ilya
use new[] to allocate, std::copy to copy, delete[] to delete
aaa
@Ilya: you can use `std::copy`. Then again, do you really need to use C-style arrays anyway or might `std::vector` not be a better solution?
Konrad Rudolph
jamesdlin
+2  A: 

If it's C++ you're writing in, then remember that C++ has new and delete for a reason. As for the question itself it depends whether you want to copy pointers or the structures themselves. If the latter, you need to copy them too!

Kornel Kisielewicz
What would make you think that the author is writing in C++?
dash-tom-bang
@dash: C++ tag in the question? ;>
Kornel Kisielewicz
+1  A: 

This is not a right way to copy even if you are working in plain C.

A pointed out in the other answer, you will end up with two(or more) struct instances pointing to same Vertext2 and GLubyte instance, which is not recommended.

This would lead to issues like who will free the memory allocate to Vertext2 GLubyte

Should I copy the arrays after copying the structure? Or is it enough just to copy the structure?

Yes this is the right way to do it

Yogesh Arora
+1  A: 

The pointers themselves will be copied, but that means that both "from" and "to" will be the same in the two sprites. You'll also need to manually allocate and copy the things pointed to by the pointers, but that implies that you also need to know how big the arrays are that are referenced by the pointers.

Note that instead of memcpy up there, you can also do '*pSpriteToReturn = *copyFromMe;' This'll copy all of the members, although if you're going to make new arrays, the only part of the tSprites that you want to actually copy is the size.

Another note would be that if your sprites always have a fixed number of vertices and vert indices, you can make those arrays inside the sprite rather than pointers. If you did this, then they would be copied correctly with both the memcpy method and the assignment that I mention in the above paragraph.

dash-tom-bang
+1  A: 

in C++ new and delete allocate on heap.

Sprite *ptr =...;
Sprite *s = new Stripe(*ptr); // copy constructor, shallow copy off pointers
s->member = new Member(*ptr->member); // copy construct sprite member

s->array = new int[4]; //allocate array
std::copy(ptr-> array, ptr->array + 4, s->array); //copy array
delete[] s->array; //delete array, must use delete[]
aaa