xyf
xyf

Reputation: 714

A potential memory overflow but not sure what's causing it

I am seeing a potential overflow: streamBuffer is a struct object (part of the FreeRTOS lib) and upon executing the following line in OutputToSerial(), I see streamBuffer.xHead's value is set to an extremely large value even though it's not being modified at the time.

LONG_TO_STR(strData, txStr);

Any way to get a better understanding of the cause? I don't see any hardfaults or anything to look in the registers...

For a reference, the following is the struct's definition:

typedef struct StreamBufferDef_t /*lint !e9058 Style convention uses tag. */
{
    volatile size_t xTail;              /* Index to the next item to read within the buffer. */
    volatile size_t xHead;              /* Index to the next item to write within the buffer. */

    size_t xLength;                     /* The length of the buffer pointed to by pucBuffer. */
    size_t xTriggerLevelBytes;          /* The number of bytes that must be in the stream buffer before a task that is waiting for data is unblocked. */
    volatile TaskHandle_t xTaskWaitingToReceive; /* Holds the handle of a task waiting for data, or NULL if no tasks are waiting. */
    volatile TaskHandle_t xTaskWaitingToSend;   /* Holds the handle of a task waiting to send data to a message buffer that is full. */
    uint8_t *pucBuffer;                 /* Points to the buffer itself - that is - the RAM that stores the data passed through the buffer. */
    uint8_t ucFlags;

    #if ( configUSE_TRACE_FACILITY == 1 )
        UBaseType_t uxStreamBufferNumber;       /* Used for tracing purposes. */
    #endif
} StreamBuffer_t;
// file.c
#define PRI_UINT64_C_Val(value) ((unsigned long) (value>>32)), ((unsigned long)value)
#define LONG_TO_STR(STR, LONG_VAL) (sprintf(STR, "%lx%lx", PRI_UINT64_C_Val(LONG_VAL)))

unsigned long long concatData(uint8_t *arr, uint8_t size)
{
    long long unsigned value = 0;
    for (uint8_t i = 0; i < size; i++)
    {
        value <<= 8;
        value |= arr[i];
    }
    return value;
}

void nRF24_ReadReg(nrfl2401 *nrf, uint8_t reg, const uint8_t rxSize, uint8_t *rxBuffer, char *text)
{
    uint8_t txBuffer[1] = {0};
    uint8_t spiRxSize = rxSize;

    if (reg <= nRF24_CMD_W_REG)
    {
        txBuffer[0] = nRF24_CMD_R_REG | (reg & nRF24_R_W_MASK);
        spiRxSize++;
    }
    else
    {
        txBuffer[0] = reg;
    }

    nRF24_SendCommand(nrf, txBuffer, rxBuffer, spiRxSize);

    OutputToSerial(txBuffer, rxBuffer, spiRxSize, text);
}

void OutputToSerial(uint8_t *writeBuffer, uint8_t *readBuffer, uint8_t size, char *text)
{
    char strData[100] = {0}, rxStrData[100] = {0};
    long long unsigned txStr = concatData(writeBuffer, size);
    long long unsigned rxStr = concatData(readBuffer, size);
    LONG_TO_STR(strData, txStr);              // POTENTIAL ERROR.....!
    LONG_TO_STR(rxStrData, rxStr);

    char outputMsg[60] = {0};
    strcpy(outputMsg, text);
    strcat(outputMsg, ":          0x%s ----------- 0x%s\n");

    printf (outputMsg, strData, rxStrData);
}

// main.c
StreamBufferHandle_t streamBuffer;

Upvotes: 1

Views: 136

Answers (1)

chux
chux

Reputation: 153407

Perhaps other issues, yet the LONG_TO_STR(x) is simply a mess.
Consider the value 0x123400005678 would print as "12345678". That ref code is broken.

Yes its too bad code has long long yet no "%llx". Easy enough to re-write it all into a clean function.

//#define PRI_UINT64_C_Val(value) ((unsigned long) (value>>32)), ((unsigned long)value)
//#define LONG_TO_STR(STR, LONG_VAL) (sprintf(STR, "%lx%lx", PRI_UINT64_C_Val(LONG_VAL)))

#include <stdio.h>
#include <string.h>
#include <limits.h>

// Good for base [2...16]
void ullong_to_string(char *dest, unsigned long long x, int base) {
  char buf[sizeof x * CHAR_BIT + 1]; // Worst case size
  char *p = &buf[sizeof buf - 1];  // set to last element
  *p = '\0';
  do {
    p--;
    *p = "0123456789ABCDEF"[x % (unsigned) base];
    x /= (unsigned) base;
  } while (x);
  strcpy(dest, p);
}

int main(void) {
  char buf[100];
  ullong_to_string(buf, 0x123400005678, 16);   puts(buf);
  ullong_to_string(buf, 0, 16);   puts(buf);
  ullong_to_string(buf, ULLONG_MAX, 16);   puts(buf);
  ullong_to_string(buf, ULLONG_MAX, 10);   puts(buf);
  return 0;
}

Output

123400005678
0
FFFFFFFFFFFFFFFF
18446744073709551615

Upvotes: 1

Related Questions