HEAP Error

[Saeed]

There is an API that I need to call to fetch a VALUE (UID)

//declaration
RET_STATUS ActivateIdle(uint8_t *pabATQ = NULL,
uint8_t *pbSAK = NULL,
uint8_t *pabUID = NULL,
uint8_t *pbUIDLength = NULL);

when the API is called with no parameters (poRd700Wrapper->ActivateIdle() ) program terminates normally but when it is used with passing the Pointers , although I get to get my value that I want (UID) returned but I get a
HEAP CORRUPTION DETECTED. CRT detected that the application wrote to memory after end of heap buffer

Its been a while since I did work with pointers in C++ but from what it seems API is allocating stoarge that its not releasing upon its return;
Am i right about this ?

		uint8_t *pabATQ= new uint8_t(0);
		uint8_t *pbSAK = new uint8_t(0);
 		uint8_t *pabUID= new uint8_t(0);
		uint8_t *pbUIDLength = new uint8_t(0);


// This Call seems to be OK
//if( ( retsReturnCode = poRd700Wrapper->ActivateIdle() ) != MI_OK )
// This Call Causes the crash.
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(pabATQ,pbSAK,pabUID,pbUIDLength))!= MI_OK)	{
        delete poRd700Wrapper;
        poRd700Wrapper = NULL;
        return NULL;
    }
	if(pbUIDLength)
	{
		if(pabUID != NULL)
		{
			uint8_t i;
			for(i=0;i<*pbUIDLength;i++)
			{
			sprintf(textUID,"%s%02x ",textUID, *pabUID+i));
			}
		}
	}

	delete pabATQ;
	delete pbSAK;
 	delete pabUID;
	delete pbUIDLength;

Is there anything obvious that I don't see ?
Cheers

[Paul McKenzie]

There is an API that I need to call to fetch a VALUE (UID)

//declaration
RET_STATUS ActivateIdle(uint8_t *pabATQ = NULL,
uint8_t *pbSAK = NULL,
uint8_t *pabUID = NULL,
uint8_t *pbUIDLength = NULL);

when the API is called with no parameters

I see no reason for you to be allocating memory yourself.

		uint8_t *pabATQ= new uint8_t(0);
		uint8_t *pbSAK = new uint8_t(0);
 		uint8_t *pabUID= new uint8_t(0);
		uint8_t *pbUIDLength = new uint8_t(0);

When a function requires a pointer, it doesn't mean it needs you to allocate memory dynamically.

All that function requires is an address of the variable.

		uint8_t pabATQ= 0;
		uint8_t pbSAK = 0;
 		uint8_t pabUID= 0;
		uint8_t pbUIDLength = 0;

if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(&pabATQ, &pbSAK, &pabUID, &pbUIDLength))!=

Then there is no need for delete.

Remove all of the unnecessary allocations and pass the address of existing variables to this function. This will remove any issue with dynamic allocation in your code. If the function still fails, then you know it has nothing to do with you're use (I would call it abuse) of dynamic memory allocation.

Second,

sprintf(textUID,"%s%02x ",textUID, *pabUID+i));

What is "textUID"? Where is its declaration? Calls to sprintf() must also be correct, or else you get memory overwrites.

Why not post a real function, and not pieces of code that aren't full functions?

Regards,

Paul McKenzie

[Saeed]

Thanks paul;
but i do need the pointer as it contains a series of Values

sprintf(textUID,"%s%02x ",textUID, *pabUID+i));

textUID is a char* with enough number of bytes allocated. That has had enough bytes allocated .
to simplify my question :
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle() ) != MI_OK ) //Caused no Error
but
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(pabATQ,pbSAK,pabUID,pbUIDLength))!= MI_OK) // causes error

		uint8_t *pabATQ= new uint8_t(0);
		uint8_t *pbSAK = new uint8_t(0);
 		uint8_t *pabUID= new uint8_t(0);
		uint8_t *pbUIDLength = new uint8_t(0);


        // Select an idle PICC (Request - Anticollision - Select).
    //if( ( retsReturnCode = poRd700Wrapper->ActivateIdle() ) != MI_OK )
	if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(pabATQ,pbSAK,pabUID,pbUIDLength))!= MI_OK)
	{
        delete poRd700Wrapper;
        poRd700Wrapper = NULL;
        return NULL;
    }
	delete pabATQ;
	delete pbSAK;
 	delete pabUID;
	delete pbUIDLength;

[Paul McKenzie]

Thanks paul;
but i do need the pointer as it contains a series of Values

sprintf(textUID,"%s%02x ",textUID, *pabUID+i));

Then your code makes no sense.

Who is supposed to allocate this buffer? You or that function you're calling? If it's you, then how do you know how much to allocate?

If it's the functions job, then the call to that function makes no sense. The only way a function can allocate memory and return it back to you is if the parameter to that function that will hold the buffer takes a reference to a pointer, or takes a pointer to a pointer. All of your parameters to that function take just a pointer, and there is no way that a function can return to you a buffer using that as the parameter.

Here is an example:

void foo(char *p)
{
    p = new char [500];
}

int main()
{
   char *myp = 0;
   foo( myp );
   // myp is still 0!  What happened?
   delete [] myp;
}

You will clearly see that myp is still 0 after the call to foo(), even though foo() allocates memory. Do you understand why, and why your function in your code could never have worked?

Regards,

Paul McKenzie

[Paul McKenzie]

textUID is a char* with enough number of bytes allocated. That has had enough bytes allocated .

Let's see the code. I see too many times when posters say something, but when you actually see the code, it is wrong or something is potentially wrong with it.

Secondly, see my simple program in my last post -- it explains as to why your function could never had worked.

Last, what about all of those other parameters that have nothing to do with buffers? All of them are dynamically allocated for no reason.

for(i=0;i<*pbUIDLength;i++)
{
	sprintf(textUID,"&#37;s%02x ",textUID, *pabUID+i));
}

There is no need for pbUIDLength to be dynamically allocated, as all it contains is the returned length. Just passing the address of an existing variable is required. A test that my company has is to see if a C++ programmer dynamically allocates for no reason if the API just needs an address. That's a sign that the person is not an experienced programmer.

Also, what is in red is the problem, as explained in my simple example. You are accessing invalid memory if pbUIDLength is > 0.

Regards,

Paul McKenzie

[Eri523]

In addition to what Paul McKenzie posted...

I have not the faintest idea of that API you're calling, but the hungarian notation name prefix pab of the parameters pabATQ and pabUID seems to mean "pointer to an array of bytes", yet you're only allocating a single byte to them. Looks suspicious to me.

[Paul McKenzie]

to simplify my question :
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle() ) != MI_OK ) //Caused no Error
but
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(pabATQ,pbSAK,pabUID,pbUIDLength))!= MI_OK) // causes error

		uint8_t *pabATQ= new uint8_t(0);
		uint8_t *pbSAK = new uint8_t(0);
 		uint8_t *pabUID= new uint8_t(0);
		uint8_t *pbUIDLength = new uint8_t(0);

If Eri523 is correct, and those are supposed to be arrays, then just declare arrays and pass the name. There is no need to dynamically allocate those either.

uint8_t pabATQ[100];
uint8_t pabUID[100];
uint8_t pbUIDLength;
//...
if( ( retsReturnCode = poRd700Wrapper->ActivateIdle(pabATQ, pbSAK,pabUID, &pbUIDLength))!= MI_OK)

Regards,

Paul McKenzie