memcpy() operation on the integer arrays in the constructor gives unexpected output in C++

Question

I know I could use std::vector in C++ instead of arrays and save me some trouble. However this question is not for practical application. Its rather for my understanding. I see '0' instead of the actual value on memcpy() operation. What am I doing wrong in this test code?

#include <stdint.h>
#include <cstring>
#include <cstdlib>
#include <iostream>

using namespace std;

class IntList
{
private:
     int* m_anList; //I could use std::vector in practical applications I know
                    //However I want to experiment what happens

public:
       IntList(const int m_anList[]){ 
       this->m_anList = new int[sizeof(m_anList+1)]; //heap allocation - since bad idea to copy on stack

       memcpy((int*)this->m_anList,m_anList,sizeof(m_anList+1)); //<-This does not look right
       cout << this->m_anList[4] << endl;//<- gives '0'??? Not expected

       }
     ~IntList(){
      if(this->m_anList)
      {
       delete[] this->m_anList; 
      }
     }

     int& operator[] (const int& nIndex);
};

int& IntList::operator[] (const int& nIndex)
{
    cout << this->m_anList[nIndex] << endl; //<- gives '0'??? Not Expected
    return this->m_anList[nIndex];
}


int main()
{

   int test_array[10] = {1,2,3,4,5,6,7,8,9};

   IntList test(test_array);

   test[2];      

   return 0;
}

I have used it on char* before and it has worked. char = 1 Byte, int = 2 Bytes but memcpy applies to void*.

Updated Code/ Solution (Thanks to Rob (who pointed out my most fundamental of several mistakes) and everyone who replied. I am not CS grad but would try to code better in the future. Thanks again.)

#include <stdint.h>
#include <cstring>
#include <cstdlib>
#include <iostream>
//#include <algorithm>
//#include <vector>

using namespace std;

class IntList
{
private:
     int* m_anList; //I could use std::vector in practical applications I know
                    //However I want to experiment what happens

public:
      IntList(const int m_anList[], std::size_t n){ 
      this->m_anList = new int[n * sizeof(int)];
      memcpy(this->m_anList,m_anList,n*sizeof(m_anList[0]));
      cout << this->m_anList[4] << endl;     
     }
     ~IntList(){
      if(this->m_anList)
       delete[] this->m_anList;
     }

     int& operator[] (const int& nIndex);
};

int& IntList::operator[] (const int& nIndex)
{
    cout << this->m_anList[nIndex] << endl;  
    return this->m_anList[nIndex];
}


int main()
{

   int hello[10] = {1,2,3,4,5,6,7,8,9};

   //cout << hello[3] << endl;

   IntList test(hello,10);

   test[2];


 return 0;
}

the best you can learn from this is that it led you to do unwholesome things that Did Not Work. now consider your future coworkers. they will foul up the same way, if you let them at raw arrays and memcpu. — Cheers and hth. - Alf, Mar 29 '12 at 19:23
What do you expect `sizeof(m_anList+1)` to be? "*int = 2 Byte*" Not on any major platform in the last 15 years... — ildjarn, Mar 29 '12 at 19:23
Not related to your question, but giving your member function parameters the same names as your member variables (m_anList) is a recipe for disaster. — Random Wombat, Mar 29 '12 at 19:32
@RandomWombat but thats why I used this-> operator to distinguish. — enthusiasticgeek, Mar 29 '12 at 19:53
@Cheersandhth.-Alf thanks I assume you are a programming stalwart and look down upon novice programmers with righteous condemnation. :) — enthusiasticgeek, Mar 30 '12 at 21:43

Robᵩ · Accepted Answer · 2012-03-29T20:56:55.620

4

sizeof(m_anList+1) doesn't do what you think it does. In particular, it is identical to sizeof(int*). Consequently, you are only allocating (typically) four or eight bytes in your new expression, rather than the size of the passed-in array.

In order to make this work, you must also pass in the size of the array:

   IntList(const int m_anList[], std::size_t n){ 
     this->m_anList = new int[n];
     memcpy(this->m_anList,m_anList,n*sizeof(m_anList[0]));
     …

edited Mar 29 '12 at 20:56

answered Mar 29 '12 at 19:23

Robᵩ

163,533
20
239
308

I had to just add Parenthesis around sizeof(int) to make it work on my G++ compiler :). – enthusiasticgeek Mar 29 '12 at 20:15
@enthusiasticgeek - Sorry. My code was untested and you found an error in it. I'm fixing it now. – Robᵩ Mar 29 '12 at 20:16
I do not have privileges yet, to edit your post so left a comment. Thanks again. – enthusiasticgeek Mar 29 '12 at 20:19
Oops, there is a bigger problem. `new` takes a number of elements, not a number of bytes. Fixing it now. – Robᵩ Mar 29 '12 at 20:56

score 2 · Answer 2 · answered Mar 29 '12 at 19:25

2

There is very much wrong with the code, including

C style casts
sizeof pointer
using memcpy instead of std::copy

No single fix is possible.

answered Mar 29 '12 at 19:25

Cheers and hth. - Alf

142,714
15
209
331

Doesn't std::copy means I need to include ? – enthusiasticgeek Mar 29 '12 at 19:37

score 1 · Answer 3 · edited May 23 '17 at 11:44

1

In C++ when declaring a function accepting a parameter using the "array syntax"

void foo(int v[])
{
    ...
}

you are indeed just declaring a function accepting a pointer. In other words the above is exactly identical to

void foo(int *v)
{
    ...
}

and sizeof(v) is the size of a pointer, not the size of an array.

Do yourself a favor and stop just trying to learn C++ by experimenting with a compiler. With the C++ language such an approach is a complete suicide.

Start instead with a good C++ book and read it cover to cover ... it's the only way.

edited May 23 '17 at 11:44

Community

1
1

answered Mar 29 '12 at 19:25

6502

112,025
15
165
265

Thanks 6502. I am learning from books authored by Scott Meyers Effective C++ and Andrei Alexandrescu Modern c++ design. I consider myself beginner in C++. But since I am not CS grad as you can tell from my code I look to experiment in coding. – enthusiasticgeek Mar 29 '12 at 19:44
Meyers Effective C++ and More Effective C++ is in my opinion very good. IMO however you should also read a basic complete description like Stroustrup TCPPPL. About Modern C++ design sure the author is very good, and also some of the ideas are eye-openeners... but the whole template machinery can be taken to a nonsense abuse level and looks more like a pointless trickery exercise than a real-world tool. Advanced metaprogramming is a very powerful and nice weapon to master, but not in C++. – 6502 Mar 29 '12 at 20:49

score 0 · Answer 4 · answered Mar 29 '12 at 19:24

Memory allocated with new does not remember its own size. You have to keep it off to the side in its own integer, and use THAT to allocate and copy the memory. Your sizeof expression will just get the number of bytes in a pointer.

And remember to implement/disable copy construction and assignment as well to avoid memory leaks.

In production code of course you should use vector which manages all these problems for you.

score 0 · Answer 5 · answered Mar 29 '12 at 19:25

0

You should put the +1 outside of the sizeof().

answered Mar 29 '12 at 19:25

Tobias

672
3
13

@enthusiasticgeek: That doesn't fix the root problem it just causes a different problem. – Blastfurnace Mar 29 '12 at 19:30

memcpy() operation on the integer arrays in the constructor gives unexpected output in C++

5 Answers5