Avoid critical sections where possible (FreeRTOS#1063)

* Avoid critical sections where possible * Repair unit test FreeRTOS_Sockets --------- Co-authored-by: Tony Josi <[email protected]>
moninom1 · Jan 2, 2024 · af07ccf · af07ccf
1 parent 10b5171
commit af07ccf
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 );