Can I delete class pointer copied using memcpy()?

Question

Is the following snippet of code valid?

class Foo
{
public:
    int a;
    SomeClass* pb;

    Foo() {...}
    Foo(const Foo& rhs)
    {
         this->a = rhs.a;
         memcpy(this->pb, rhs.pb, sizeof(SomeClass));
    }
    ~Request()
    {
        delete this->pb; // Suppose pb is a result of copy constructor,
                         // Is this valid or is there a better way?
    }
};

score 3 · Answer 1 · answered Apr 28 '11 at 14:52

3

You can't copy classes using memcpy. And memcpy is just copying memory, not allocating anything.

answered Apr 28 '11 at 14:52

Šimon Tóth

35,456
20
106
151

also, using "memcpy" to copy the memory, without new allocation, will lead to memory issues (double free, etc.), in the normal pace of using the objects and freeing it. suggested is to use copy-constructors way. – parasrish Feb 09 '16 at 16:29

score 3 · Accepted Answer · answered Apr 28 '11 at 14:56

3

You haven't actually allocated anything for pb. If you allocate it with new, then using delete in the destructor is perfectly valid.

But why oh why are you doing a memcpy? You should be using the copy constructor of the class and letting it copy itself in the appropriate fashion. The only time a memcpy would be remotely acceptable is when the class is a simple struct of POD types.

answered Apr 28 '11 at 14:56

Mark Ransom

299,747
42
398
622

The reason to why I would like to use memcpy is because the `pb` is actually a pointer to a derived class of `SomeClass`. `SomeClass` and its derived classes is not under my control, and Foo might not know the exact type of `pb`. Do you have any suggestions on this problem? – avee Apr 28 '11 at 15:04
Well you can't use `memcpy`. What's wrong with `this->pb = new SomeClass(*rhs.pb);`? – Šimon Tóth Apr 28 '11 at 15:07
@Let_Me_Be: is it valid if we don't know the exact type of rhs.pb? It might be a type of `SomeClassWhichIsDerivedFromSomeClass` – avee Apr 28 '11 at 15:09
@avee That's irrelevant when you are copying `SomeClass`. – Šimon Tóth Apr 28 '11 at 15:12
@avee, to do this properly `SomeClass` must define some kind of virtual `clone` method that will allocate and copy itself at the most derived level. If you don't have that I'm afraid you're stuck - there's no good way to do it. – Mark Ransom Apr 28 '11 at 15:12
@Let_Me_Be, read up on the slicing problem: http://stackoverflow.com/questions/274626/what-is-the-slicing-problem-in-c – Mark Ransom Apr 28 '11 at 15:14
2

@avee See this topic: http://stackoverflow.com/questions/5731217/c-how-to-copy-create-derived-class-instance-from-a-pointer-to-a-polymorphic-ba – RedX Apr 28 '11 at 15:15
@Mark Uh? But he wants to copy SomeClass not the derived class. – Šimon Tóth Apr 28 '11 at 15:15
@Let_Me_Be: I actually want it to be a base class pointer to whatever type it actually is. @Mark Ransom @RedX: Thanks! I will accept this answer since it answers all my questions. – avee Apr 28 '11 at 15:38

score 1 · Answer 3 · answered Apr 28 '11 at 14:58

It's valid, as in it will compile. But it is stricken with errors. You never allocated any memory, so you'd need to do that first. But there is absolutely no reason to use memcpy, and in most cases, it does the wrong thing. Just do this:

Foo(const Foo& rhs)
    :a(rhs.a),
     pb(new SomeClass(*rhs.pb))
{    
}

Or better yet, use a smart pointer, and you don't have to worry about deleting. Or better yet, don't use a pointer at all if you retain sole ownership of the object.

That's a good way to do it, but it doesn't solve the actual problem that I am having, see my comment on @Mark Ransom's answer — avee, Apr 28 '11 at 15:05

score 1 · Answer 4 · answered Apr 28 '11 at 15:00

This code snippet is wrong because you can't call delete without calling to new for allocation of memory.

Also your memcpy call can cause segmentation fault in most cases, because you are copying to unintialized pointer. You should allocate memort for SomeClass pointer before calling to memcpy.

score 1 · Answer 5 · answered Apr 28 '11 at 15:07

Using memcpy with anything other than POD structures/objects ("plain old data"--things with no non-default constructors or destructors, and with virtual methods) is a recipe for disaster. If you fix your program so that you're using memcpy to copy a POD structure which contains an "ordinary" pointer to a class item, any pointers copied as part of that structure must be handled the same as pointers copied via other means. Exactly one copy (or the original) of any such pointer must be "deleted"--no more and no less. If you copy the pointer and abandon the original without calling "delete" on it, you should then call "delete" on exactly one copy.

In general, one should avoid using memcpy in C++; in C, structures never had any "hidden" information which could be left in an inconsistent state by memcpy, but in C++ they often do (with the exception of POD structures). Although C++ is designed to be mostly compatible with C, the fact that something is legal in C does not mean that it is proper in C++.

razlebe · Answer 6 · 2011-04-28T15:11:38.053

You haven't allocated memory for that pointer using new, so you absolutely shouldn't be using delete on it. memcpy isn't allocating any memory for you.

Specifically, the only pointers you should be invoking delete on are pointers that have previously been returned by new, or a null pointer. To use delete on any other pointer is undefined behaviour, according to the C++03 standard, 3.7.3.2/3-4.

The value of the first argument supplied to one of the deallocation functions provided in the standard library may be a null pointer value; if so, the call to the deallocation function has no effect. Otherwise, the value supplied to operator delete(void*) in the standard library shall be one of the values returned by a previous invocation of either operator new(size_t) or operator new(size_t, const std::nothrow_t&)

...[snip]...

The effect of using an invalid pointer value (including passing it to a deallocation function) is undefined.)

Can I delete class pointer copied using memcpy()?

6 Answers6