From af07ccf4c4ed038409910892c400b3e04b40c183 Mon Sep 17 00:00:00 2001 From: Hein Tibosch Date: Tue, 2 Jan 2024 14:37:53 +0800 Subject: [PATCH] Avoid critical sections where possible (#1063) * Avoid critical sections where possible * Repair unit test FreeRTOS_Sockets --------- Co-authored-by: Tony Josi --- source/FreeRTOS_Sockets.c | 4 +- source/FreeRTOS_UDP_IPv4.c | 10 +-- source/FreeRTOS_UDP_IPv6.c | 10 +-- .../FreeRTOS_Sockets_UDP_API_utest.c | 72 +++++++++++++++---- 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/source/FreeRTOS_Sockets.c b/source/FreeRTOS_Sockets.c index ef07f4335..ca170e050 100644 --- a/source/FreeRTOS_Sockets.c +++ b/source/FreeRTOS_Sockets.c @@ -1171,7 +1171,7 @@ static NetworkBufferDescriptor_t * prvRecvFromWaitForPacket( FreeRTOS_Socket_t c if( lPacketCount > 0 ) { - taskENTER_CRITICAL(); + vTaskSuspendAll(); { /* The owner of the list item is the network buffer. */ pxNetworkBuffer = ( ( NetworkBufferDescriptor_t * ) listGET_OWNER_OF_HEAD_ENTRY( &( pxSocket->u.xUDP.xWaitingPacketsList ) ) ); @@ -1183,7 +1183,7 @@ static NetworkBufferDescriptor_t * prvRecvFromWaitForPacket( FreeRTOS_Socket_t c ( void ) uxListRemove( &( pxNetworkBuffer->xBufferListItem ) ); } } - taskEXIT_CRITICAL(); + ( void ) xTaskResumeAll(); } *pxEventBits = xEventBits; diff --git a/source/FreeRTOS_UDP_IPv4.c b/source/FreeRTOS_UDP_IPv4.c index 9562ea90d..0cea8ae5b 100644 --- a/source/FreeRTOS_UDP_IPv4.c +++ b/source/FreeRTOS_UDP_IPv4.c @@ -445,13 +445,9 @@ BaseType_t xProcessReceivedUDPPacket_IPv4( NetworkBufferDescriptor_t * pxNetwork { vTaskSuspendAll(); { - taskENTER_CRITICAL(); - { - /* Add the network packet to the list of packets to be - * processed by the socket. */ - vListInsertEnd( &( pxSocket->u.xUDP.xWaitingPacketsList ), &( pxNetworkBuffer->xBufferListItem ) ); - } - taskEXIT_CRITICAL(); + /* Add the network packet to the list of packets to be + * processed by the socket. */ + vListInsertEnd( &( pxSocket->u.xUDP.xWaitingPacketsList ), &( pxNetworkBuffer->xBufferListItem ) ); } ( void ) xTaskResumeAll(); diff --git a/source/FreeRTOS_UDP_IPv6.c b/source/FreeRTOS_UDP_IPv6.c index 7644780da..894f863f3 100644 --- a/source/FreeRTOS_UDP_IPv6.c +++ b/source/FreeRTOS_UDP_IPv6.c @@ -536,13 +536,9 @@ BaseType_t xProcessReceivedUDPPacket_IPv6( NetworkBufferDescriptor_t * pxNetwork { vTaskSuspendAll(); { - taskENTER_CRITICAL(); - { - /* Add the network packet to the list of packets to be - * processed by the socket. */ - vListInsertEnd( &( pxSocket->u.xUDP.xWaitingPacketsList ), &( pxNetworkBuffer->xBufferListItem ) ); - } - taskEXIT_CRITICAL(); + /* Add the network packet to the list of packets to be + * processed by the socket. */ + vListInsertEnd( &( pxSocket->u.xUDP.xWaitingPacketsList ), &( pxNetworkBuffer->xBufferListItem ) ); } ( void ) xTaskResumeAll(); diff --git a/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_UDP_API_utest.c b/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_UDP_API_utest.c index 6f6ac2779..d321d4999 100644 --- a/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_UDP_API_utest.c +++ b/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_UDP_API_utest.c @@ -380,9 +380,14 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_JustUDPHeader( void ) listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); - uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + + uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -441,9 +446,13 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_Packet100( void ) listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); - uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -503,9 +512,13 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_Packet100SizeSmall( void listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); - uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -566,7 +579,13 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_Packet100SizeSmall_Peek( listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + } + + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -624,7 +643,12 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_Packet100SizeSmall_Peek_ listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -681,9 +705,15 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_Packet100SizeSmall_ZeroC listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + + uxListRemove_ExpectAndReturn( &xNetworkBuffer.xBufferListItem, 0U ); + } - uxListRemove_ExpectAndReturn( &xNetworkBuffer.xBufferListItem, 0U ); + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -733,7 +763,11 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBegining_Packet100SizeSmall_Zero xListItem.pvOwner = &xNetworkBuffer; xSocket->u.xUDP.xWaitingPacketsList.xListEnd.pxNext = &xListItem; - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv4_HEADER ); @@ -789,9 +823,13 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_IPv6Packet100( void ) listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); - uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( ipSIZE_OF_IPv6_HEADER ); @@ -850,9 +888,13 @@ void test_FreeRTOS_recvfrom_BlockingGetsPacketInBetween_UnknownIPHeaderSize( voi listCURRENT_LIST_LENGTH_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), 0x12 ); - listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); + vTaskSuspendAll_Expect(); + { + listGET_OWNER_OF_HEAD_ENTRY_ExpectAndReturn( &( xSocket->u.xUDP.xWaitingPacketsList ), &xNetworkBuffer ); - uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + uxListRemove_ExpectAndReturn( &( xNetworkBuffer.xBufferListItem ), 0 ); + } + xTaskResumeAll_ExpectAndReturn( pdFALSE ); uxIPHeaderSizePacket_IgnoreAndReturn( 0xFF );