0

I need to resize the m_Array and it is saying problems in valgrind when adding 11. element m_Max = 10 m_Len = 10 I am allowed to use cstdio,cstdlib,cstring and iostream.

/* m_Len is length of array and m_Max is maximal length */
resizing in CAccount::NewAccount

if ( m_Len >= m_Max )
{
     if (m_Max == 0) m_Max = 5;
     m_Max *= 2;
     CAccount * tmp = new CAccount[m_Max];
     memcpy ( tmp, m_Array, m_Len * sizeof(CAccount));
     delete[] m_Array;
     m_Array = tmp; 
}
m_Array[m_Len] = x;
m_Len++;

here is aditional code which is involved in this:

CAccount
{
 public:
  ~CAccount ( void )
  {
    delete[] m_Trans;
  }
private:
 struct Transaction
 {
   int a;
   const char * b;
   const char * c;
 }
Transaction * m_Trans;


}


CAccount * m_Array;

this is what valgrind says

my couts...

==2458== Invalid read of size 4
==2458==    at 0x8048D67: CAccount::AddTransaction(int const&, char const*, char const*) (bank_test.cpp:121)
==2458==    by 0x8049645: CBank::Transaction(char const*, char const*, int, char const*) (bank_test.cpp:268)
==2458==    by 0x8049946: main (bank_test.cpp:327)
==2458==  Address 0x434b150 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 
==2458== Invalid free() / delete / delete[] / realloc()
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048D9A: CAccount::AddTransaction(int const&, char const*, char const*) (bank_test.cpp:121)
==2458==    by 0x8049645: CBank::Transaction(char const*, char const*, int, char const*) (bank_test.cpp:268)
==2458==    by 0x8049946: main (bank_test.cpp:327)
==2458==  Address 0x434b150 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 
==2458== Invalid read of size 4
==2458==    at 0x8048D67: CAccount::AddTransaction(int const&, char const*, char const*) (bank_test.cpp:121)
==2458==    by 0x8049678: CBank::Transaction(char const*, char const*, int, char const*) (bank_test.cpp:269)
==2458==    by 0x8049946: main (bank_test.cpp:327)
==2458==  Address 0x434b188 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 
==2458== Invalid free() / delete / delete[] / realloc()
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048D9A: CAccount::AddTransaction(int const&, char const*, char const*) (bank_test.cpp:121)
==2458==    by 0x8049678: CBank::Transaction(char const*, char const*, int, char const*) (bank_test.cpp:269)
==2458==    by 0x8049946: main (bank_test.cpp:327)
==2458==  Address 0x434b188 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 

then it writes another couts and then it says this

==2458== Invalid read of size 4
==2458==    at 0x8048AD7: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804922E: CBank::~CBank() (bank_test.cpp:224)
==2458==    by 0x80499F0: main (bank_test.cpp:381)
==2458==  Address 0x434b348 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 
==2458== Invalid free() / delete / delete[] / realloc()
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804922E: CBank::~CBank() (bank_test.cpp:224)
==2458==    by 0x80499F0: main (bank_test.cpp:381)
==2458==  Address 0x434b348 is 0 bytes inside a block of size 4 free'd
==2458==    at 0x402B598: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2458==    by 0x8048B0A: CAccount::~CAccount() (bank_test.cpp:92)
==2458==    by 0x804951E: CBank::NewAccount(char const*, int) (bank_test.cpp:254)
==2458==    by 0x804991B: main (bank_test.cpp:323)
==2458== 
==2458== 
==2458== HEAP SUMMARY:
==2458==     in use at exit: 0 bytes in 0 blocks
==2458==   total heap usage: 17 allocs, 27 frees, 1,268 bytes allocated
==2458== 
==2458== All heap blocks were freed -- no leaks are possible
==2458== 
==2458== For counts of detected and suppressed errors, rerun with: -v
==2458== ERROR SUMMARY: 20 errors from 6 contexts (suppressed: 0 from 0)
  • Can you use `boost`? If so, have a look at [boost.container](http://www.boost.org/doc/libs/1_55_0/doc/html/container/main_features.html#container.main_features.containers_of_incomplete_types), particularly `boost::container::vector`. – juanchopanza Apr 12 '14 at 16:51
  • 2
    **Don't** `memcpy()` C++ objects. Use `std::copy()` instead. The exception is if the object is a POD-type, which your's clearly isn't. And you may want to review [The Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). – WhozCraig Apr 12 '14 at 16:51

1 Answers1

3

This is what happens:

CAccount * tmp = new CAccount[m_Max];

You created 20 new objects CAccount.

memcpy ( tmp, m_Array, m_Len * sizeof(CAccount));

You memcpy-ied the content of the 10 old objects CAccount to the new array.

The problem is, that as you used just memcpy and no copy constructor that would take care of the ownership of the data, both the new and old versions of the objects are pointing to the same m_Trans data.

Once you call delete[] m_Array; the destructors on the old objects are called, and they remove the m_Trans

delete[] m_Trans;

Now the new objects are pointing to m_Trans data that were already removed so accessing the pointer will lead to undefined behavior.

The cleanest way to solve this is to use is to instead of

memcpy ( tmp, m_Array, m_Len * sizeof(CAccount));

do

std::copy(m_Array, m_Array + m_Len, tmp)

This calls the opreator= on the new objects, so you need to make that correctly as well:

CAccount
{
public:
  CAccount::CAccount(const CAccount& account)
  : m_trans(<initialisation code here>)
{
  <Copy the data from the account>
}

This solution is cleanest but sub-optimal, as you are copying the data, you could just switch the owner of the data, but if you are not shooting for performance this is better.

kovarex
  • 1,766
  • 18
  • 34
  • Oh, thank you I will try to implement that. I thought that memcpy makes the binary copy of the data not just another pointer to it. Hope it will work then I tell. – user3527230 Apr 12 '14 at 21:02
  • 1
    You got me wrong, the memcpy really makes binary copy of the data. but the binary data of CAccount contains the pointer. In other words, memcpy is shallow copy. If you wanted to have the deep copy automated, you would need to use the C++ structures for containers, like vectors.http://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy – kovarex Apr 13 '14 at 13:47
  • For posterity: Assuming C++11, `std::move(m_Array, m_Array + m_Len, tmp)` is where it is at. Or, preferably, use `vector`. – Xarn Aug 21 '14 at 10:52