Will this C++ code cause a memory leak (casting array new)
The behaviour of the code is undefined. You may be lucky (or not) and it may work with your compiler, but really that's not correct code. There's two problems with it:
- The
delete
should be an arraydelete []
. - The
delete
should be called on a pointer to the same type as the type allocated.
So to be entirely correct, you want to be doing something like this:
delete [] (BYTE*)(pStruct);
Technically I believe it could cause a problem with mismatched allocators, though in practice I don't know of any compiler that would not do the right thing with this example.
More importantly if STRUCT
where to have (or ever be given) a destructor then it would invoke the destructor without having invoked the corresponding constructor.
Of course, if you know where pStruct came from why not just cast it on delete to match the allocation:
delete [] (BYTE*) pStruct;
Yes it will cause a memory leak.
See this except from C++ Gotchas: http://www.informit.com/articles/article.aspx?p=30642 for why.
Raymond Chen has an explanation of how vector new
and delete
differ from the scalar versions under the covers for the Microsoft compiler... Here:
http://blogs.msdn.com/oldnewthing/archive/2004/02/03/66660.aspx
IMHO you should fix the delete to:
delete [] pStruct;
rather than switching to malloc
/free
, if only because it's a simpler change to make without making mistakes ;)
And, of course, the simpler to make change that I show above is wrong due to the casting in the original allocation, it should be
delete [] reinterpret_cast<BYTE *>(pStruct);
so, I guess it's probably as easy to switch to malloc
/free
after all ;)
I personally think you'd be better off using std::vector
to manage your memory, so you don't need the delete
.
std::vector<BYTE> backing(sizeof(STRUCT) + nPaddingSize);
STRUCT* pStruct = (STRUCT*)(&backing[0]);
Once backing leaves scope, your pStruct
is no longer valid.
Or, you can use:
boost::scoped_array<BYTE> backing(new BYTE[sizeof(STRUCT) + nPaddingSize]);
STRUCT* pStruct = (STRUCT*)backing.get();
Or boost::shared_array
if you need to move ownership around.