CCS/LAUNCHXL-CC26X2R1: Freeing a dynamically allocated variable

Shyam Shankar79

Part Number: LAUNCHXL-CC26X2R1
Other Parts Discussed in Thread: SYSBIOS

Tool/software: Code Composer Studio

Hello All,

I created a Queue to store the data coming from an SPI master device. So I dynamically created a queue variable to enque the data into the queue as shown below :

spiQueue *DataQueue; // Contains : elem, spiData

DataQueue = (spiQueue *)malloc( sizeof(spiQueue) ) ;
memset(DataQueue->spiData, 0, 264);
memcpy( DataQueue->spiData, data, dataLen );
Queue_enqueue(QueueHandle, &DataQueue->elem);

//free(DataQueue);

Now, when I try to free the DataQueue here, then the data becomes inaccessible later on. So my question is, is it not required to free the dynamically allocated DataQueue after enqeueuing it?

Also I believe I am getting an GAP_EVT_INSUFFICIENT_MEMORY issue due to not freeing this data. Could that be ?

Regards,

Shyam

over 6 years ago

0 Tomasz Kocon over 6 years ago

Guru 26200 points

Shyam Shankar79 said:
Now, when I try to free the DataQueue here, then the data becomes inaccessible later on. So my question is, is it not required to free the dynamically allocated DataQueue after enqeueuing it?

A total amount of memory which can be dynamically allocated is limited by a heap size. If you do not free the unused memory blocks, time would come that the system would be unable to allocate a new memory block to your programme.
It is not required to free the unused memory, however it could bring a system to a malfunctioning state.
When you do free(), your data is not erased. The memory block after freeing could be assigned to the other purposes later on.

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi,
But why does the data become inaccessible later on when I free it as in my previous post. Once it is in the queue, can't we free the variable?

0 Keith Barkley over 6 years ago in reply to Shyam Shankar79

Guru 42225 points

Which variable do you want to free?

free(DataQueue) will free the malloc'd memory *pointed* to by DataQueue. That will mark the malloc'd block as being available to the OS to use for other purposes.

You should only free a block of memory when you no longer need it. Your culprit might be QueueEnqueue(). It could very well not copy the contents of DataQueue to the queue, but just a pointer to it. On an MCU with limited cycles and RAM, that would be the most efficient solution.

0 Keith Barkley over 6 years ago in reply to Shyam Shankar79

Guru 42225 points

From the docs:
"The ti.sysbios.knl.Queue module provides support for creating lists of objects. A Queue is implemented
as a doubly-linked list, so that elements can be inserted or removed from anywhere in the list, and so that
Queues do not have a maximum size."

This strongly implies that the Queue functions simply manipulate pointers to objects, not copying the objects themselves.

0 ToddMullanix over 6 years ago in reply to Keith Barkley

TI__Guru* 96960 points

Correct. The Queue module does not copy any data. When you place an item onto a Queue instance, it cannot be delete. You can only delete it after it is removed from the queue.

0 Shyam Shankar79 over 6 years ago in reply to ToddMullanix

Expert 2330 points

Hi All,

Thanks for the replies. So, I understand that the "DataQueue" variable can only be "freed" using free(), once it has been dequeued/read from the queue. That does clear my query about why I was unable to use that data further on after freeing it.

But, then how would I work with my scenario here :

I have a function that is called every time I get a data from SPI. I receive a data at time t1, calls the function which then dynamically allocate DataQueue and then put it into mySpiQueue. Then I get another data at time t2, I again call the function which again dynamically allocate DataQueue and put it into the queue. Then if I were to read/dequeue the data at a later time t3. How would I free the previously allocated variables ?

I now included freeing up the variabLes that are allocated during the dequeueing. But I believe that would not free the variable dynamically allocated at the time of enqueuing.

At dequeue :

spiQueue *spiReadQueue = Queue_dequeue(spiQueueHandle);

//Do some operations

free(spiReadQueue);

I have been working with the ble_multirole project as my base. In it too, I don't see where they are freeing the variables that were allocated during the enqueueing . Also, referencing to my first post, would the GAP_EVT_INSUFFICIENT_MEMORY be due to not freeing up the dynamically allocated variables.

Regards,

Shyam

0 Tomasz Kocon over 6 years ago in reply to Shyam Shankar79

Guru 26200 points

Hi,

before I address your questions from the last post,
please explain what you code does:

1: spiQueue *DataQueue; // Contains : elem, spiData
2:
3: DataQueue = (spiQueue *)malloc( sizeof(spiQueue) ) ;
4: memset(DataQueue->spiData, 0, 264);
5: memcpy( DataQueue->spiData, data, dataLen );
6: Queue_enqueue(QueueHandle, &DataQueue->elem);

Line 4: where is DataQueue->spiData pointing to?

0 Kristopher Burney over 6 years ago in reply to Shyam Shankar79

TI__Intellectual 1975 points

Hello Shyam,

Shyam Shankar79 said:
Also, referencing to my first post, would the GAP_EVT_INSUFFICIENT_MEMORY be due to not freeing up the dynamically allocated variables.

GAP_EVT_INSUFFICIENT_MEMORY could indeed be caused because you are not freeing up the heap. It more than likely is the cause. If your application is not freeing up memory, eventually the stack will not have any heap space to allocate for itself. By freeing up the spiQueue * after you have de-queued it and processed the message, you will release that block of memory back to the heapmgr (the os) and allow something else to use the space. (Either the stack or your application).

Shyam Shankar79 said:
How would I free the previously allocated variables ?

It is easiest to study how the multi_role application is performing it's message passing. You can do it the same way.

In the multi_role.c Searching for enqueue will show you a function called Util_enqueueMsg().

uint8_t Util_enqueueMsg(Queue_Handle msgQueue,
                        Event_Handle event,
                        uint8_t *pMsg)
{
  queueRec_t *pRec;

  // Allocated space for queue node.
#ifdef USE_ICALL
  if ((pRec = ICall_malloc(sizeof(queueRec_t))))
#else
  if ((pRec = (queueRec_t *)malloc(sizeof(queueRec_t))))
#endif
  {
    pRec->pData = pMsg;

    // This is an atomic operation
    Queue_put(msgQueue, &pRec->_elem);

    // Wake up the application thread event handler.
    if (event)
    {
      Event_post(event, UTIL_QUEUE_EVENT_ID);
    }

    return TRUE;
  }

  // Free the message.
#ifdef USE_ICALL
  ICall_free(pMsg);
#else
  free(pMsg);
#endif

  return FALSE;
}

Here you can see a malloc for the space needed. Then it adds the element into the queue. Then it posts an event so that the application can process the data when there is time to do so.

Right below this function there is a matching Util_dequeueMsg() function.

uint8_t *Util_dequeueMsg(Queue_Handle msgQueue)
{
  queueRec_t *pRec = Queue_get(msgQueue);

  if (pRec != (queueRec_t *)msgQueue)
  {
    uint8_t *pData = pRec->pData;

    // Free the queue node
    // Note:  this does not free space allocated by data within the node.
#ifdef USE_ICALL
    ICall_free(pRec);
#else
    free(pRec);
#endif

    return pData;
  }

  return NULL;
}

Here you can see it pulls the msg from the top of the queue, saves the data* , frees the queue* and passes the data * as the return value.

Here it is important to note that this works because the mrEvt_t * is malloced as well before from multi_role_enqueueMsg().

I would suggest you take another look at the sequence of message passing in the application.

The basic sequence goes like this.

multi_role_enqueueMsg() -> Util_enqueueMsg() ... Util_dequeueMsg() -> multi_role_processAppMsg()

Best,

Kris

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi Tomasz,
DataQueue-> spiData points to spiData withing the spiQueue variable. Thank you for the reply. I believe Kristopher has provided a solution to my case.

Regards,
Shyam

0 Shyam Shankar79 over 6 years ago in reply to Kristopher Burney

Expert 2330 points

Hi Kristopher,
I too have done something similar. I malloced a variable and then put it into the queue using enqueue. Then raised an event for processing at a later time. Then when pulling from the queue, I used "spiQueue *spiReadQueue = Queue_dequeue(spiQueueHandle);". Then after processing, I freed spiReadQueue. So, I believe that I have freed the variables now, since spiReadQueue would in fact be pointing to the same memory location as is pointed by the variable used at the time of putting into the queue. Hence, I would have freed the memory used by the heap.

Regards,
Shyam

0 Tomasz Kocon over 6 years ago in reply to Shyam Shankar79

Guru 26200 points

Hi Shyam,

Shyam Shankar79 said:
DataQueue-> spiData points to spiData withing the spiQueue variable.

I do not agree with you.
DataQueue-> spiData points to data which is referred by spiQueue->spiData pointer.
Issue is that this pointer is uninitialized and your memset() statement sets to 0s an unknown region of memory.

This memset () leads to an unpredictable behavior of the program.

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi Tomasz,
But I have malloced DataQueue. Only after that, am I using memset to set it to 0. So doesn't it point to a specific memory?

Regards,
Shyam

0 Tomasz Kocon over 6 years ago in reply to Shyam Shankar79

Guru 26200 points

You do not set the newly allocated DataQueue.
Your newly allocated DataQueue is a pointer to spiQueue and is assigned by malloc();
However, spiQueue structure, pointed by your DataQueue pointer, has unknown values.
You assign sizeof(spiQueue) number of bytes to zeroes starting from address pointed by DataQueue->spiData and
this DataQueue->spiData pointer has unknown value.

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi Tomasz,

So are you saying that I should avoid the memset I have used there?

Regards,

Shyam

0 Tomasz Kocon over 6 years ago in reply to Shyam Shankar79

Guru 26200 points

Hi Shyam,

Shyam Shankar79 said:
So are you saying that I should avoid the memset I have used there?

DataQueue->spiData is big issue. spiData pointer has unknown value.
Both memset() and memcpy() write into undetermined memory address.

Your example code, sooner rather then later would corrupt memory.
You need to address this issue.

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi Tom,

Below is my actual code :

-----------------------------------------------------------------------------------------------------------------------

typedef struct spiQueue
{
Queue_Elem elem;
uint8_t spiData[MAX_SPI_MSGLEN];
}spiQueue;

spiQueue *spiDataQueue;

spiDataQueue = (spiQueue *)malloc( sizeof(spiQueue) ) ;
memset(spiDataQueue->spiData, 0, MAX_SPI_MSGLEN);
spiDataQueue->spiData[SPI_QUEUE_CMDPOS] = cmd;
// Loading the length of payload data
spiDataQueue->spiData[SPI_QUEUE_LENPOS] = dataLen;
memcpy(&(spiDataQueue->spiData[SPI_QUEUE_DATAPOS]), data, dataLen);
Queue_enqueue(spiQueueHandle, &spiDataQueue->elem);

-----------------------------------------------------------------------------------------------------------------------

When you say that both memset and memcpy would cause an issue, how would I then actually put some data into this queue then?

0 Keith Barkley over 6 years ago in reply to Shyam Shankar79

Guru 42225 points

I am not sure where Tomasz is coming from. He was thinking that you just had a pointer, not an array. I was wondering how he could be so dogmatic without seeing the code. You are fine.

0 Kristopher Burney over 6 years ago in reply to Keith Barkley

TI__Intellectual 1975 points

Shyam,

I concur with Keith here. I believe Tomasz assumed as such due to the example code I provided earlier as well as the existing code base the starting example provides. You have modified the queue element in such a way which would appear fine with with the code you have now shown.

Of course, Tomasz, if I'm characterizing your thoughts please correct me!

Best,
Kris

0 Tomasz Kocon over 6 years ago in reply to Kristopher Burney

Guru 26200 points

To all,

Shyam, you are fine with a structure containing a table.
Seeing memset() immediately followed by memcpy() I had some remarks about performance.
Somehow I have lost the context of your issue.
Sorry!
Glad you did.

0 Shyam Shankar79 over 6 years ago in reply to Tomasz Kocon

Expert 2330 points

Hi Tomasz,

Thank you for the responses. Hope to find you again when the next issue pops up for me :)

Regards,
Shyam

Code Composer Studio™︎

Code Composer Studio forum

CCS/LAUNCHXL-CC26X2R1: Freeing a dynamically allocated variable