17

I have a class that wraps a vector:

template <typename T>
class PersistentVector
{
  private:
    std::vector<T> values;
  public:
    std::vector<T> & getValues() throw () { return values; }
....
}

The instance is:

PersistentVector<double> moments;

I am trying to make a copy of all the doubles into a buffer allocated by somebody else. Here is where I create the buffer:

// invariant: numMoments = 1
double * data_x = new double[numMoments];

Here is my attempt at copying the contents of the vector into the buffer:

double theMoment = moments.getValues()[0];
// theMoment = 1.33

std::memcpy(
  data_x, 
  &(moments.getValues().operator[](0)), 
  numMoments * sizeof(double));
// numMoments = 1

double theReadMoment = data_x[0];
// theReadMoment = 6.9533490643693675e-310

As you can see, I am getting a garbage value from the buffer. Why is this so?

Working solution

Use std::copy (thanks to WhozCraig)

double theMoment = moments.getValues()[0];
// theMoment = 1.33

std::copy(moments.getValues().begin(), moments.getValues().end(), data_x);

double theReadMoment = data_x[0];
// theReadMoment = 1.33

Failed solution

Try data() instead of operator[]()

double theMoment = moments.getValues()[0];
// theMoment = 1.33 

std::memcpy(
  data_x, 
  moments.getValues().data(),
  numMoments * sizeof(double));
// numMoments = 1

double theReadMoment = data_x[0];
// theReadMoment = 6.9533490643693675e-310

Still curious as to why my original code is failing!

  • Just so you know, vector has a method called "data" which returns the data pointer. – Ben Mar 24 '14 at 06:34
  • 4
    `std::copy(moments.getValues().begin(), moments.getValues().end(), data_x)` ? Or did I miss something obvious? (and fyi, things like "invariant: numMoments = 1" are *not* comforting to see if that really is a pointer to a *single* double). – WhozCraig Mar 24 '14 at 06:37
  • @WhozCraig Do you suggest that because 1. C functions suck and 2. I suck at using C functions? I'm afraid it's 1 and 2. –  Mar 24 '14 at 06:38
  • `numMoments` could be a very large number. I made it 1 for a simple test case. –  Mar 24 '14 at 06:39
  • OK, I fixed the problem using `std::copy`. For educative purposes (and best answer), can anybody spot the error in my use of `std::memcpy`? –  Mar 24 '14 at 06:40
  • I suggest it because `double` isn't always the thing you'll be copying for the rest of your career, and when its a vector of complex *objects* (appropriately coded to support copying) `std::copy` will *just work*. The implementation should have specializations for trivial types to just do what you're trying to (i.e. it will degenerate to a memcpy anyway, but its not a terrible idea regardless to dance with the one you brought (i.e. the C++ standard library)). – WhozCraig Mar 24 '14 at 06:41
  • 3
    Try replacing .operator[](0) with .data(). – Ben Mar 24 '14 at 06:41
  • Does memcpy call copy constructors? – Ben Mar 24 '14 at 06:42
  • 1
    @Ben **no**, it does *not*. – WhozCraig Mar 24 '14 at 06:43
  • @Ben To get `&(moments.getValues().data())` or `moments.getValues().data()` ? The first is an error: "lvalue required as unary ‘&’ operand" and the second does not fix the garbage value. –  Mar 24 '14 at 06:43
  • 1
    @Ben if not a trivial type, *yes* (or assignment operators, etc). If the target isn't something like an insert or pushback iterator, assignments are uncoiled (usually), otherwise emplacement is generally the enjoyed optimization. – WhozCraig Mar 24 '14 at 06:44
  • @jakeliquorblues Yeah, the second one. – Ben Mar 24 '14 at 06:44
  • 1
    @WhozCraig Why not making your comments an answer? It's a common pitfall and elaboration would certainly be valuable to many users. BTW, the problem would be bigger and harder to debug if the memcpy was performed in the opposite way - from standard array to vector (memory corruption). – SomeWittyUsername Mar 24 '14 at 06:57
  • @icepack that would *suck*. I'm just got back from another question and i see new content added. One of them i don't see how it doesn't work. The wild-card seems to be `numMoments`, which leads me to believe perhaps that thing is not holding the value it should be. Now I'm curious to know if the OP changed that failed attempt byte count to simply `sizeof(double) * moments.getValues().size()` rather than the `numMoments` calculation being used. – WhozCraig Mar 24 '14 at 07:03
  • @WhozCraig, do you mean the "Failed Solution" or the OPs original when you say one of them not working surprises you? Also, I respectfully second the request to post an answer to this if you have figured this one out. – batbrat Mar 24 '14 at 12:45

1 Answers1

20

I tried doing this in 3 different ways and all of them worked. The data() method is the way to do it right. If it does not work - check if the data was corrupted in some way.

 std::vector<double> vec1 = {1.33,2.66,3.99};
 double* vec2 = new double[3];

 int numMoments = 1;

 ::memcpy(
  vec2, 
  vec1.data(),
  numMoments * sizeof(double));

 ::memcpy(
  vec2, 
  &(*vec1.begin()),
  numMoments * sizeof(double));

 ::memcpy(
  vec2, 
  &(vec1[0]),
  numMoments * sizeof(double));
vmax33
  • 643
  • 5
  • 13
  • In the second memcpy, you use & and * together. You can equivalently write ::memcpy(ve2, vec1.begin(), numMoments * sizeof(double)); AFAIK. – batbrat Mar 24 '14 at 12:43
  • 6
    @batbrat It doesn't work the way you suggessted, because begin returns iterator which is not convertible to void*. – vmax33 Mar 24 '14 at 14:21
  • 2
    I facepalmed after reading your comment. Not my finest moment. Thanks for reminding me. – batbrat Mar 24 '14 at 14:54