fix(esp_ringbuf): Fixed a bug where in a no-split buffer received items prematurely

This commit fixes a bug in the no-split buffer which could receive an
item prematurely if the space on the buffer is acquired until the buffer
is full. The commit also adds a unit test for this scenario.

Closes https://github.com/espressif/esp-idf/issues/14568
This commit is contained in:
Sudeep Mohanty 2024-09-16 13:10:42 +02:00
parent 3c3d05e97b
commit 7d442f76f1
2 changed files with 87 additions and 4 deletions

View File

@ -506,6 +506,13 @@ static BaseType_t prvCheckItemAvail(Ringbuffer_t *pxRingbuffer)
return pdFALSE; //Byte buffers do not allow multiple retrievals before return
}
if ((pxRingbuffer->xItemsWaiting > 0) && ((pxRingbuffer->pucRead != pxRingbuffer->pucWrite) || (pxRingbuffer->uxRingbufferFlags & rbBUFFER_FULL_FLAG))) {
// If the ring buffer is a no-split buffer, the read pointer must point to an item that has been written to.
if ((pxRingbuffer->uxRingbufferFlags & (rbBYTE_BUFFER_FLAG | rbALLOW_SPLIT_FLAG)) == 0) {
ItemHeader_t *pxHeader = (ItemHeader_t *)pxRingbuffer->pucRead;
if ((pxHeader->uxItemFlags & rbITEM_WRITTEN_FLAG) == 0) {
return pdFALSE;
}
}
return pdTRUE; //Items/data available for retrieval
} else {
return pdFALSE; //No items/data available for retrieval
@ -989,9 +996,6 @@ BaseType_t xRingbufferSendAcquire(RingbufHandle_t xRingbuffer, void **ppvItem, s
if (xItemSize > pxRingbuffer->xMaxItemSize) {
return pdFALSE; //Data will never ever fit in the queue.
}
if ((pxRingbuffer->uxRingbufferFlags & rbBYTE_BUFFER_FLAG) && xItemSize == 0) {
return pdTRUE; //Sending 0 bytes to byte buffer has no effect
}
return prvSendAcquireGeneric(pxRingbuffer, NULL, ppvItem, xItemSize, xTicksToWait);
}

View File

@ -806,6 +806,7 @@ TEST_CASE("Test ring buffer ISR", "[esp_ringbuf][qemu-ignore]")
* Every permutation of core pinning of the sending and receiving task will be
* tested.
*/
#if !CONFIG_FREERTOS_UNICORE
#define SRAND_SEED 3 //Arbitrarily chosen srand() seed
#define SMP_TEST_ITERATIONS 4
@ -1021,6 +1022,7 @@ TEST_CASE("Test static ring buffer SMP", "[esp_ringbuf]")
cleanup();
}
#endif
#endif //!CONFIG_FREERTOS_UNICORE
#if !CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH && !CONFIG_RINGBUF_PLACE_ISR_FUNCTIONS_INTO_FLASH
/* -------------------------- Test ring buffer IRAM ------------------------- */
@ -1030,7 +1032,7 @@ static IRAM_ATTR __attribute__((noinline)) bool iram_ringbuf_test(void)
bool result = true;
uint8_t item[4];
size_t item_size;
RingbufHandle_t handle = xRingbufferCreate(CONT_DATA_TEST_BUFF_LEN, RINGBUF_TYPE_NOSPLIT);
RingbufHandle_t handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT);
result = result && (handle != NULL);
spi_flash_guard_get()->start(); // Disables flash cache
@ -1123,3 +1125,80 @@ TEST_CASE("Test ringbuffer with caps", "[esp_ringbuf]")
// Free the ring buffer
vRingbufferDeleteWithCaps(rb_handle);
}
/* ---------------------------- Test no-split ring buffer SendAquire and SendComplete ---------------------------
* The following test case tests the SendAquire and SendComplete functions of the no-split ring buffer.
*
* The test case will do the following...
* 1) Create a no-split ring buffer.
* 2) Acquire space on the buffer to send an item.
* 3) Send the item to the buffer.
* 4) Verify that the item is received correctly.
* 5) Acquire space on the buffer until the buffer is full.
* 6) Send the items out-of-order to the buffer.
* 7) Verify that the items are not received until the first item is sent.
* 8) Send the first item.
* 9) Verify that the items are received in the correct order.
*/
TEST_CASE("Test no-split buffers always receive items in order", "[esp_ringbuf]")
{
// Create buffer
RingbufHandle_t buffer_handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT);
TEST_ASSERT_MESSAGE(buffer_handle != NULL, "Failed to create ring buffer");
// Acquire space on the buffer to send an item and write to the item
void *item1;
TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &item1, MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
*(uint32_t *)item1 = 0x123;
// Send the item to the buffer
TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, item1));
// Verify that the item is received correctly
size_t item_size;
uint32_t *received_item = xRingbufferReceive(buffer_handle, &item_size, TIMEOUT_TICKS);
TEST_ASSERT_NOT_NULL(received_item);
TEST_ASSERT_EQUAL(item_size, MEDIUM_ITEM_SIZE);
TEST_ASSERT_EQUAL(*(uint32_t *)received_item, 0x123);
// Return the space to the buffer after receiving the item
vRingbufferReturnItem(buffer_handle, received_item);
// At this point, the buffer should be empty
UBaseType_t items_waiting;
vRingbufferGetInfo(buffer_handle, NULL, NULL, NULL, NULL, &items_waiting);
TEST_ASSERT_MESSAGE(items_waiting == 0, "Incorrect items waiting");
// Acquire space on the buffer until the buffer is full
#define MAX_NUM_ITEMS ( BUFFER_SIZE / ( MEDIUM_ITEM_SIZE + ITEM_HDR_SIZE ) )
void *items[MAX_NUM_ITEMS];
for (int i = 0; i < MAX_NUM_ITEMS; i++) {
TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &items[i], MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
TEST_ASSERT_NOT_NULL(items[i]);
*(uint32_t *)items[i] = (0x100 + i);
}
// Verify that the buffer is full by attempting to acquire space for another item
void *another_item;
TEST_ASSERT_EQUAL(pdFALSE, xRingbufferSendAcquire(buffer_handle, &another_item, MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
// Send the items out-of-order to the buffer. Verify that the items are not received until the first item is sent.
// In this case, we send the items in the reverse order until the first item is sent.
for (int i = MAX_NUM_ITEMS - 1; i > 0; i--) {
TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, items[i]));
TEST_ASSERT_NULL(xRingbufferReceive(buffer_handle, &item_size, 0));
}
// Send the first item
TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, items[0]));
// Verify that the items are received in the correct order
for (int i = 0; i < MAX_NUM_ITEMS; i++) {
received_item = xRingbufferReceive(buffer_handle, &item_size, TIMEOUT_TICKS);
TEST_ASSERT_EQUAL(*(uint32_t *)received_item, (0x100 + i));
vRingbufferReturnItem(buffer_handle, received_item);
}
// Cleanup
vRingbufferDelete(buffer_handle);
}