Using memcpy to copy an array of objects

Question

Recently I have encountered the error double free or corruption error in my project. After some test runs , the problem is pinned down to the copy function which uses memcpy.

  class Pen
  {    string make;
       string model;
       string color;
       public:
       Pen();
  }

  class A
  { private:

    Pen* array; //an array that stores pen objects 
    int NumOfItem;   
    int Maxsize;        
    void CopyArray(const A& source);

    public:
    A();
    A(const A& source);//copy constructor where uses the CopyArray private mentioned below
    ~A();
  }






  void A::CopyArray(const A& source)
  {       
    memcpy(array, source.array, len * sizeof(Pen));//
    return;
  }

  void A::A(const A& source)//copy constructor that performs a deep copy from B
  {    array = new Pen[source.NumOfItem];
       NumOfItem = source.NumOfItem;
       MaxisIze=source.Maxize;
       CopyArray(source);
  }

When I change my code and use for loop to copy each parameter, it works. I am still trying to understand why memcpy is causing the problem if all it does is copying all the data bitwise to the new object......(Sorry for the messy format..)

Rather use [`std::copy()`](http://en.cppreference.com/w/cpp/algorithm/copy) than `memcopy()`, Binary copies of complex objects don't work well. — πάντα ῥεῖ, May 22 '15 at 00:39
Memcpy mindlessly copies a block of data. If the data contains a pointer, the pointer is copied, not the data at the pointer. If you try to access the pointed at memory later, it may be gone because the object that owns that block of memory has done away with it or been destroyed itself. — user4581301, May 22 '15 at 00:40

score 13 · Accepted Answer · answered May 22 '15 at 00:44

13

The problem with using memcpy is that it bypasses copy constructors. This is OK only when your class is composed of primitives.

However, Pen class has non-primitive data members of type std::string. These objects require a call of copy constructor to be copied. memcpy does not perform any calls to copy constructors, which leads to internal representations of std::string becoming shared, which in turn causes undefined behavior on destruction.

Copying with a loop, on the other hand, invokes a copy constructor, so your code runs without a problem.

C++ Standard Library provides a utility function for copying ranges called std::copy. Using this function avoids the problem that you see, because it invokes copy constructors as needed.

answered May 22 '15 at 00:44

Sergey Kalinichenko

714,442
84
1,110
1,523

What do you mean by undefined behavior on destruction? – GalaxyVintage May 22 '15 at 01:29
4

@Lzy `std::string` objects have pointers inside to reference the actual data inside the string. These pointers get deallocated on destruction. When you destroy `Pen`, all its strings get destroyed as well. After `memcpy` two instances of `std::string` point to the same memory. When copied `Pen` gets destroyed, the pointers in the original become invalid, and vice verse, depending on what gets destroyed first. When the other object gets destroyed, it submits invalid pointers to operator `delete[]`, which is undefined behavior that often causes a program crash. – Sergey Kalinichenko May 22 '15 at 01:34
I think I got it now . Thanks for the help – GalaxyVintage May 22 '15 at 02:11
also what is primitive data and how to distinguish them from non-primitive data? – GalaxyVintage May 22 '15 at 02:18
2

@Lzy `int`s, `long`s, `float`s, `double`s, etc. are primitive data types. classes and `struct`s composed of them (and/or other `struct`s composed of primitives) can be copied with `memcpy`. This is generalized by the concept of "trivial copying", explained at the link in the other answer. – Sergey Kalinichenko May 22 '15 at 02:26

Bill Lynch · Answer 2 · 2015-05-22T00:52:14.947

You can only use memcpy() to copy objects that are trivially copyable. Let's see if Pen meets this requirement.

#include <string>
using namespace std;

class Pen {
    string make;
    string model;
    string color;
    public:
    Pen();
};

static_assert(std::is_trivially_copyable<Pen>::value, "Pen is not trivially copyable");

When compiled, this will return the error:

blah.cc:12:1: error: static_assert failed "Pen is not trivially copyable"
static_assert(std::is_trivially_copyable<Pen>::value, "Pen is not trivially copyable");
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So, we can clearly see that Pen is not trivially copyable, so we can't use memcpy() with it. You should probably use std::copy instead.

Using memcpy to copy an array of objects

2 Answers2