• Locked Locked
  • Replies 1 reply
  • Subscribers 97 subscribers
  • Views 343 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Implementation of UTIL_allocMem

Marco Braga
Expert 1780 points

It seems to me that the current implementation of UTIL_allocMem in util.c has several flaws. This is the current implementation:


// Allocate memory from the ad-hoc heap
void *UTIL_allocMem(Uint32 size)
{
  void *cPtr;
  Uint32 size_temp;

  // Ensure word boundaries
  size_temp = ((size + 4) >> 2 ) << 2;
 
  if((currMemPtr + size_temp) > ((Uint32) &EXTERNAL_RAM_END))
  {
    return NULL;
  }

  cPtr = (void *) (((Uint32) &EXTERNAL_RAM_START) + currMemPtr);
  currMemPtr += size_temp;

  return cPtr;
}

 

First problem is in the code used to check that the requested alloc size is aligned to 4:

size_temp = ((size + 4) >> 2 ) << 2;

It works in it main purpose, i.e. size_temp is always a multiple of 4, but it has a side effect of increasing the size of the allocated mem even if the requested size is actually already a multiple of 4. As an example if size = 4, size_temp is 8. This causes a problem when trying to "deallocate" memory using UTIL_setCurrMemPtr, such as in UBL and several other programs. Check for example nandboot.c:

rxBuf = (Uint8*)UTIL_allocMem((APP_IMAGE_SIZE>>1));

...

UTIL_setCurrMemPtr((void *)((Uint32)UTIL_getCurrMemPtr() - (APP_IMAGE_SIZE>>1)));

 

The problem is that the real allocated size is (APP_IMAGE_SIZE>>1) + 4, even if (APP_IMAGE_SIZE>>1) is divisible by 4. While this method of deallocating has flaws too if the size is not multiple of 4, it does not work correctly even when (like in the case of UBL) the size is a multiple of 4.

Second problem is in the code used in UTIL_allocMem to check if the size of requested memory is not bigger than available external memory:

  if((currMemPtr + size_temp) > ((Uint32) &EXTERNAL_RAM_END))

But "currMemPtr" does not actually start from EXTERNAL_RAM_START, instead it starts from 0 and EXTERNAL_RAM_START is added later in the code when calculating the value of cPtr.

The check should instead be something similar to:

  if((currMemPtr + size_temp) > ((Uint32) &EXTERNAL_RAM_END) - (Uint32) &EXTERNAL_RAM_START))

While both problems do not cause big problems in the actual use, they could be fixed very simply...

 

  • Expert 1780 points

    I forgot to add my proposed implementation:

    // Allocate memory from the ad-hoc heap
    void UTIL_freeMem(Uint32 size)
    {
      Uint32 size_temp;

      // Ensure word boundaries
      size_temp = (size + 3) & ~3;
     
      if(size_temp <= currMemPtr)
          currMemPtr -= size_temp;
    }


    // Allocate memory from the ad-hoc heap
    void *UTIL_allocMem(Uint32 size)
    {
      void *cPtr;
      Uint32 size_temp;

      // Ensure word boundaries
      size_temp = (size + 3) & ~3;
     
      if((currMemPtr + size_temp) > ((Uint32) &EXTERNAL_RAM_END - (Uint32) &EXTERNAL_RAM_START))
      {
        return NULL;
      }

      cPtr = (void *) (((Uint32) &EXTERNAL_RAM_START) + currMemPtr);
      currMemPtr += size_temp;

      return cPtr;
    }

    I've added a simple implementation of "UTIL_freeMem". Of course it has the same limits of the "UTIL_setCurrMemPtr" method (i.e. UTIL_allocMem and UTIL_freeMem must follow a strict LIFO order) but at least does not cause "leaks" when the requested size is not multiple of 4.