Problem with malloc, realloc, _msize and memcpy

[Bren]

Can anybody tell me what is wrong with this code?

void SystemModule::AddModuleDependency(PTR_MODULE_INFO pModuleInfo)
{
if (!pModuleInfo)
return;

PTR_MODULE_INFO pDest = 0;

if (!m_pDependencies)
{
unsigned int nNewSize = sizeof(MODULE_INFO);
m_pDependencies = (PTR_MODULE_INFO)malloc(nNewSize);
pDest = m_pDependencies;
}
else
{
unsigned int nCurSize = _msize(m_pDependencies);
unsigned int nNewSize = nCurSize +
sizeof(MODULE_INFO);

m_pDependencies =
(PTR_MODULE_INFO)realloc(m_pDependencies, nNewSize);

pDest = m_pDependencies;
pDest += nCurSize;
}

memcpy(pDest, pModuleInfo, sizeof(MODULE_INFO));
}

In the second block, when m_pDependencies is already allocated, in
appears that when nCurSize is added to pDest, the pointer is set much
further ahead in memory than it should. The MODULE_INFO is relatively
small (84 bytes) but it goes 6000+ bytes beyond m_pDependencies. The
the memcpy gets an access violation.

When debugging, I can manually change the pDest address after the add,
then the memcpy works, but the next time through the block,
_msize(m_pDependencies) fails.

Any suggestions?

 

[John Harrison]

Can anybody tell me what is wrong with this code?

void SystemModule::AddModuleDependency(PTR_MODULE_INFO pModuleInfo)
{
if (!pModuleInfo)
return;

PTR_MODULE_INFO pDest = 0;

if (!m_pDependencies)
{
unsigned int nNewSize = sizeof(MODULE_INFO);
m_pDependencies = (PTR_MODULE_INFO)malloc(nNewSize);
pDest = m_pDependencies;
}
else
{
unsigned int nCurSize = _msize(m_pDependencies);
unsigned int nNewSize = nCurSize +
sizeof(MODULE_INFO);

m_pDependencies =
(PTR_MODULE_INFO)realloc(m_pDependencies, nNewSize);

pDest = m_pDependencies;
pDest += nCurSize;
}

memcpy(pDest, pModuleInfo, sizeof(MODULE_INFO));
}

In the second block, when m_pDependencies is already allocated, in
appears that when nCurSize is added to pDest, the pointer is set much
further ahead in memory than it should. The MODULE_INFO is relatively
small (84 bytes) but it goes 6000+ bytes beyond m_pDependencies. The
the memcpy gets an access violation.

When debugging, I can manually change the pDest address after the add,
then the memcpy works, but the next time through the block,
_msize(m_pDependencies) fails.

Any suggestions?

You've forgotten that

pDest += n;

does not add n to pDest but n*sizeof(MODULE_INFO), pointer arithmetic is the
name for this.

From what I can see of your code you are manipulating a dynamic array of
MODULE_INFO. Have you considered using std::vector<MODULE_INFO>? Its much
less messy and standard as well.

The use of _msize is bad, since its is not standard and in any case does not
return the size you used to allocate the memory block (which is what you are
assuming I think).

I.e. after

char* ptr = malloc(10);
unsigned int size = _msize(ptr);

size will not necessarily equal 10.

john

 

[Bren]

You've forgotten that

pDest += n;

does not add n to pDest but n*sizeof(MODULE_INFO), pointer arithmetic is the
name for this.

<forehead smack>

You're right. I *always* forgot the sizeof(ptr) is the size of the
memory pointed to and not the pointer. Thanks!

From what I can see of your code you are manipulating a dynamic array of
MODULE_INFO. Have you considered using std::vector<MODULE_INFO>? Its much
less messy and standard as well.

Wouldn't a std::vector<PTR_MODULE_INFO> be better?

This memory is passed out of a DLL through a struct, so I'm not sure
if a vector will get over. But I will give it a shot.

The use of _msize is bad, since its is not standard and in any case does not
return the size you used to allocate the memory block (which is what you are
assuming I think).

I.e. after

char* ptr = malloc(10);
unsigned int size = _msize(ptr);

size will not necessarily equal 10.

Holy smokes izzat right? Isn't that supposed to be how it works? Why
and under what conditions would it not return 10?

 

[Sam Holden]

Holy smokes izzat right? Isn't that supposed to be how it works? Why
and under what conditions would it not return 10?

Malloc will return a pointer to an allocated block of memory of *at least*
10 bytes (or NULL).

Well, I suspect anyway. I've never used anything remotely like _msize...

 

[Gianni Mariani]

Sam Holden wrote:
....

Well, I suspect anyway. I've never used anything remotely like _msize...

And you probably should avoid using _msize in the future.


It'd decidedly non-portable.

 

[Sam Holden]

Sam Holden wrote:
...

And you probably should avoid using _msize in the future.


It'd decidedly non-portable.

I guessed that much.

It seems decidedly useless to me as well, anyway

 

[Ivan Vecerina]

Wouldn't a std::vector<PTR_MODULE_INFO> be better?

It depends on the relative cost of copying an instance of MODULE_INFO,
and how well the final/maximum size of the vector can be predicted.

This memory is passed out of a DLL through a struct, so I'm not sure
if a vector will get over. But I will give it a shot.

Note that you can use a std::vector for storage, but still use a pointer
to the vector's first element to pass the contents to a structure:
callMyDllFunc( & myVector.front(), myVector.size() );

The elements of an std::vector are guaranteed to be allocated contiguously
(not formally in the C++98 std yet, but this is being corrected).

hth,

 

[Bren]

Yep,

Figured out how to use a list in this situation, much nicer!

And no _msize!!!

Thanks all,

Bren

 

[Stephen Howe]

I.e. after

Holy smokes izzat right? Isn't that supposed to be how it works? Why
and under what conditions would it not return 10?

When you request a size of 10 bytes from malloc, there is a certain amount
of "bookkeeping" necessary by the heap manager to keep track of the block of
memory allocated. It marks the block as "in use". So it may well be that
that actual block is 16 bytes in size say, of which some memory is for the
bookkeeping infomation and possibly some padding. When you later call
free(), the heap manager will typically amalgamate the memory with other
blocks if it works out that the surrounding memory is managed by the heap
and those blocks are currently free.

In most implementations _msize() will the total size of the block, not the
size requested from malloc() which will be smaller.

As others have pointed out _msize() is not much use. If you want to
"remember" the size you allocated, you have to separately keep track.
_msize() may be of use if you were writing a heap utility that dumped the
state of the heap. You would have to know something about your compiler
vendors implementation.

Stephen Howe