Char buffer corruption within FreeRTOS queue messages

Kernel
1

rksheth wrote on Tuesday, July 28, 2015:

Some info first:
HW: STM32F407 Discovery board
Compiler: gcc-arm-none-eabi-4_8-2014q2
Yes, demo code (and a fair bit of my code) runs fine on this device.

I’m implementing a USART trace logging feature to be used in a multi-task environment. The idea is that multiple tasks can send a message to the “debugQueue” and the “debugTask” will sequentially service them and write them out over USART. In order to implement this, I created a queue of the following type:

typedef struct{
uint16_t size;
trace_level_t level;
char buffer[UART_BUF_SIZE];
} debug_queue_msg_t;

I have tested the basic UART code and know that it works correctly.

What I am finding is that the strings that I place in the message buffer come out garbled when they are received by the debugTask.

Here’s an example:

I have inspected the message in GDB immediately before sending:
xQueueSend(debugQueue, &msg, 0);

and immediately after receiving it in the debugTask:
xQueueReceive(debugQueue, &msg, portMAX_DELAY );

Former:
(gdb) p msg
$2 = {size = 33, level = TRACE_MED, buffer = “SPEED HARD SET TO 0.75 DUTY CYCLE”, ‘\245’ <repeats 67 times>}

Latter:
(gdb) p msg
$3 = {size = 33, level = TRACE_MED,
buffer = “S”, ‘\245’ <repeats 72 times>, “\375\377\377\377\000\000\000\000”, ‘\245’ <repeats 16 times>, “\275\024”}

As you can see, the first character is still present but the rest of the string has been overwritten.

Not sure if I’m missing something obvious or if there’s something I don’t understand about using queues with FreeRTOS.

I should also point out that I’m aware that placing a 100 byte buffer within the queue message is not ideal - but I’m not sure whether there’s a fundamental issue with that or if it’s just bad practice because introduces some extra writes.

Any advice is appreciated!

2

richard_damon wrote on Tuesday, July 28, 2015:

Could your receiving task be overruning its stack? That is a somewhat large object if the receiving task is using the stack to hold it.

I am also a bit concerned that the incoming message isn’t null terminated. This means you need to be very careful to not use any routines (like strcpy) to move the string around.

3

rtel wrote on Tuesday, July 28, 2015:

The queue should work whatever the size of the item being queued, provided you have created the queue correctly so each item is actually that size. As you say yourself, queuing something that big is not ideal, normally for something so large you would just queue a pointer to it - but then you have to ensure the string still exists in the buffer being pointed to at the time it is printed out.

To add to Richard Damon’s comment on stack overflow, do you have configCHECK_FOR_STACK_OVERFLOW set to 2? Also, do you have configASSERT() defined?

Regards.

4

rksheth wrote on Wednesday, August 05, 2015:

Appreciate the input.

@ Richard - I do in fact explicitly insert a null character as the last char in the buffer, and I memcpy the entire buffer, as opposed to doing a strcpy. It’s inefficient, but should be safe. I also use strnlen for sizes elsewhere - moral of the story, I think I’m covered from null character related issues.

@ RTE - Config assert is defined and the check for stack oveflow variable is set to 2. I was also concerned about stack overflow, so I changed the stack size of the task from min stack size (260 in the config file I have) to 1024, but saw the same behavior. I should note that I didn’t see my code enter the stack overflow handler.

I did consider passing a pointer, but in order to guarantee that the buffer with my string would not be overwritten, I realized I’d need to create one of the following:

  1. A secondary shared circular buffer with a mutex on it, which can cause tasks to block
  2. A secondary shared circular buffer with a new task servicing it with a queue, effectively recreating my debug task
  3. Provide a circular buffer to each task that uses the debug interface, which seems messy and not very scalable. Plus, then I have to think about my buffer size with respect to my message volume in addition to my queue size.

In conclusion, limiting my message size and keeping the buffer in the queue message seems the most elegant solution, even if it does consume some more cycles.

If you have any recommendations on other experiments I can try to narrow down why that buffer is getting corrupted, I would welcome them.

Thanks!

5

heinbali01 wrote on Thursday, August 06, 2015:

Hi,

You can either pass the messages by value, and create a big message queue like this:

/* Just some examples of sizes */
#define UART_BUF_SIZE   64
#define DEBUG_MSG_COUNT	100

typedef struct{
    uint16_t size;
    trace_level_t level;
    char buffer[UART_BUF_SIZE];
} debug_queue_msg_t;

/* The size of this queue is big: 100 time the size of
'debug_queue_msg_t', approximately 6800 bytes: */

xQueueCreate( DEBUG_MSG_COUNT, sizeof( debug_queue_msg_t ) );

Or you can pass the message by reference (memory pointer), and create it like this:


#define DEBUG_MSG_COUNT	100

/* This queue will only store and forward objects
of the size of a pointer.
This size of this queue is only 100 * 4 = 400 bytes: */
xQueueCreate( DEBUG_MSG_COUNT, sizeof( debug_queue_msg_t * ) )

It looks like you mixed things up: your queue has an element size of 4 bytes and you expect it will hold complete message of type ‘debug_queue_msg_t’, is that true?

Regards.

6

heinbali01 wrote on Thursday, August 06, 2015:

Some addition to my previous post:

FreeRTOS Queues are used for several purposes, for example:

  • Semaphores: nothing is being stored and the element size is zero
  • Token queues: the element size equals 1 and characters are being sent
  • Messages queues: the element size equals to the size of a memory pointer

I would not recommend using queues to store large objects as in my previous post.

If you pass pointers you must make sure who is the owner of the pointers at any time. A safe and easy way is to use 2 queues:

    void vLoggingInit( void )
    {
    BaseType xIndex;
    static debug_queue_msg_t messages[ DEBUG_MSG_COUNT ];

        /* The queue that will be consumed by the UART task. */
        xLoggingQueue = xQueueCreate( DEBUG_MSG_COUNT, sizeof( debug_queue_msg_t * ) );

        /* The queue that holds free messages. */
        xFreeQueue = xQueueCreate( DEBUG_MSG_COUNT, sizeof( debug_queue_msg_t * ) );
        for( xIndex = 0; xIndex < DEBUG_MSG_COUNT; xIndex++ )
        {
            debug_queue_msg_t *pxPointer = messages + xIndex;
            xQueueSendToBack( xFreeQueue, ( void *)&pxPointer, 0 );
        }
    }
    void vSendLogging( trace_level_t xLevel, const char *pcFormat, ... )
    {
        debug_queue_msg_t *pxPointer;
        if( xQueueReceive( xFreeQueue, ( void *)&pxPointer, 0ul ) != pdFALSE )
        {
        va_list vArgs;

            /* pxPointer is ours now, fill the fields. */
            pxPointer->level = xLevel;
            va_start (vArgs, pcFormat);
            pxPointer->size = vsnprintf( pxPointer->buffer, sizeof pxPointer->buffer, pcFormat, vArgs );
            va_end (vArgs);
            /* The parameter 'xTicksToWait' can safely be set at the maximum because
            xLoggingQueue is created big enough to to hold ALL messages: */
            xQueueSendToBack( xLoggingQueue, ( void *)&pxPointer, portMAX_DELAY );
        }
        else
        {
            /* The UART task is a bit slow, all messages are occupied. */
        }
    }
    void vReceiveLogging( ... )
    {
        debug_queue_msg_t *pxPointer;
        if( xQueueReceive( xLoggingQueue, ( void *)&pxPointer, 0ul ) != pdFALSE )
        {
            /* Send the message: */
            uart_send( pxPointer );
            /* Pass the pointer to the poll of free pointers: */
            xQueueSendToBack( xFreeQueue, ( void *)&pxPointer, portMAX_DELAY );
        }
    }

The above function vSendLogging() can be rewritten as an ISR version but I would not recommend to do so: it will influence the process too much.

Note: I didn’t take the time to test the above code, excuse me if it contains typos or some mistake :slight_smile:

Regards.