From d3ce35f78b1197e6f794473b2ffaa99246f525d8 Mon Sep 17 00:00:00 2001 From: Hein Tibosch Date: Wed, 4 Oct 2023 14:49:57 +0800 Subject: [PATCH 1/2] New helper function: FreeRTOS_get_tx_base (#544) * IPv4/single: new function: FreeRTOS_get_tx_base * Changed some code comments and repaired a typo. * Attempt to repair utest * Changes after CI checks * utest: Added tests for get_tx_base * Do not use const socket type in FreeRTOS_get_tx_base() * Removed comments from cmake file ut * Repaired UT * Removed a nested if/endif couple --------- Co-authored-by: Hein Tibosch --- source/FreeRTOS_Sockets.c | 49 ++++++++++++++- source/include/FreeRTOS_Sockets.h | 16 +++-- .../FreeRTOS_Sockets_TCP_API_utest.c | 63 +++++++++++++++++++ test/unit-test/FreeRTOS_Sockets/ut.cmake | 14 +++++ 4 files changed, 134 insertions(+), 8 deletions(-) diff --git a/source/FreeRTOS_Sockets.c b/source/FreeRTOS_Sockets.c index 1419e9a82..85b6efe31 100644 --- a/source/FreeRTOS_Sockets.c +++ b/source/FreeRTOS_Sockets.c @@ -4364,7 +4364,49 @@ void vSocketWakeUpUser( FreeRTOS_Socket_t * pxSocket ) #if ( ipconfigUSE_TCP == 1 ) /** - * @brief Get a direct pointer to the circular transmit buffer. + * @brief Get a direct pointer to the beginning of the circular transmit buffer. + * + * @param[in] xSocket: The socket owning the buffer. + * + * @return Address the first byte in the circular transmit buffer if all checks pass. + * Or else, NULL is returned. + */ + uint8_t * FreeRTOS_get_tx_base( Socket_t xSocket ) + { + uint8_t * pucReturn = NULL; + FreeRTOS_Socket_t * pxSocket = ( FreeRTOS_Socket_t * ) xSocket; + + /* Confirm that this is a TCP socket before dereferencing structure + * member pointers. */ + if( prvValidSocket( pxSocket, FREERTOS_IPPROTO_TCP, pdFALSE ) == pdTRUE ) + { + StreamBuffer_t * pxBuffer = pxSocket->u.xTCP.txStream; + + /* If the TX buffer hasn't been created yet, + * and if no malloc error has occurred on this socket yet. */ + if( ( pxBuffer == NULL ) && + ( pxSocket->u.xTCP.bits.bMallocError == pdFALSE_UNSIGNED ) ) + { + /* Create the outgoing stream only when it is needed */ + ( void ) prvTCPCreateStream( pxSocket, pdFALSE ); + pxBuffer = pxSocket->u.xTCP.txStream; + } + + if( pxBuffer != NULL ) + { + pucReturn = pxBuffer->ucArray; + } + } + + return pucReturn; + } +#endif /* ipconfigUSE_TCP */ +/*-----------------------------------------------------------*/ + +#if ( ipconfigUSE_TCP == 1 ) + +/** + * @brief Get a direct pointer to the TX head of the circular transmit buffer. * * @param[in] xSocket The socket owning the buffer. * @param[in] pxLength This will contain the number of bytes that may be written. @@ -4387,7 +4429,10 @@ void vSocketWakeUpUser( FreeRTOS_Socket_t * pxSocket ) { pxBuffer = pxSocket->u.xTCP.txStream; - if( ( pxBuffer == NULL ) && ( pxSocket->u.xTCP.bits.bMallocError != pdTRUE_UNSIGNED ) ) + /* If the TX buffer hasn't been created yet, + * and if no malloc error has occurred on this socket yet. */ + if( ( pxBuffer == NULL ) && + ( pxSocket->u.xTCP.bits.bMallocError == pdFALSE_UNSIGNED ) ) { /* Create the outgoing stream only when it is needed */ ( void ) prvTCPCreateStream( pxSocket, pdFALSE ); diff --git a/source/include/FreeRTOS_Sockets.h b/source/include/FreeRTOS_Sockets.h index 5296098c9..6ae8fdc86 100644 --- a/source/include/FreeRTOS_Sockets.h +++ b/source/include/FreeRTOS_Sockets.h @@ -325,15 +325,12 @@ BaseType_t FreeRTOS_shutdown( Socket_t xSocket, BaseType_t xHow ); - #if ( ipconfigUSE_TCP == 1 ) - /* Release a TCP payload buffer that was obtained by * calling FreeRTOS_recv() with the FREERTOS_ZERO_COPY flag, * and a pointer to a void pointer. */ - BaseType_t FreeRTOS_ReleaseTCPPayloadBuffer( Socket_t xSocket, - void const * pvBuffer, - BaseType_t xByteCount ); - #endif /* ( ipconfigUSE_TCP == 1 ) */ + BaseType_t FreeRTOS_ReleaseTCPPayloadBuffer( Socket_t xSocket, + void const * pvBuffer, + BaseType_t xByteCount ); /* Returns the number of bytes available in the Rx buffer. */ BaseType_t FreeRTOS_rx_size( ConstSocket_t xSocket ); @@ -367,6 +364,13 @@ /* For internal use only: return the connection status. */ BaseType_t FreeRTOS_connstatus( ConstSocket_t xSocket ); +/* For advanced applications only: + * Get a direct pointer to the beginning of the circular transmit buffer. + * In case the buffer was not yet created, it will be created in + * this call. + */ + uint8_t * FreeRTOS_get_tx_base( Socket_t xSocket ); + /* For advanced applications only: * Get a direct pointer to the circular transmit buffer. * '*pxLength' will contain the number of bytes that may be written. */ diff --git a/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_TCP_API_utest.c b/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_TCP_API_utest.c index e59769711..bd88e0cd1 100644 --- a/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_TCP_API_utest.c +++ b/test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_TCP_API_utest.c @@ -1826,3 +1826,66 @@ void test_prvTCPSendLoop_NullBuffer() TEST_ASSERT_EQUAL( uxDataLength, xReturn ); } + +/** + * @brief Invalid parameters passed to the function. + */ +void test_FreeRTOS_get_tx_base_InvalidParams( void ) +{ + uint8_t * pucReturn; + FreeRTOS_Socket_t xSocket; + BaseType_t xLength; + size_t uxLength = 128; + size_t uxMallocSize; + StreamBuffer_t * pxBuffer; + + memset( &xSocket, 0, sizeof( xSocket ) ); + xSocket.u.xTCP.uxTxStreamSize = uxLength; + + /* Invalid Protocol. */ + pucReturn = FreeRTOS_get_tx_base( &xSocket ); + TEST_ASSERT_EQUAL( NULL, pucReturn ); + + /* NULL socket. */ + pucReturn = FreeRTOS_get_tx_base( NULL ); + TEST_ASSERT_EQUAL( NULL, pucReturn ); + +/* FAIL: Memory Mismatch. Byte 0 Expected 0xB0 Was 0xE0. */ +/* Function pvPortMalloc Argument xSize. Function called with unexpected argument value. */ + + /* Add an extra 4 (or 8) bytes. */ + uxLength += sizeof( size_t ); + + /* And make the length a multiple of sizeof( size_t ). */ + uxLength &= ~( sizeof( size_t ) - 1U ); + + uxMallocSize = ( sizeof( *pxBuffer ) + uxLength ) - sizeof( pxBuffer->ucArray ); + + pvPortMalloc_ExpectAndReturn( uxMallocSize, NULL ); + + vTCPStateChange_Expect( &xSocket, eCLOSE_WAIT ); + + /* NULL stream. */ + xSocket.ucProtocol = FREERTOS_IPPROTO_TCP; + pucReturn = FreeRTOS_get_tx_base( &xSocket ); + TEST_ASSERT_EQUAL( NULL, pucReturn ); +} + +/** + * @brief All fields of the socket are NULL. + */ +void test_FreeRTOS_get_tx_base_AllNULL( void ) +{ + uint8_t * pucReturn; + FreeRTOS_Socket_t xSocket; + uint8_t ucStream[ ipconfigTCP_MSS ]; + + memset( &xSocket, 0, sizeof( xSocket ) ); + memset( ucStream, 0, ipconfigTCP_MSS ); + + xSocket.ucProtocol = FREERTOS_IPPROTO_TCP; + xSocket.u.xTCP.txStream = ( StreamBuffer_t * ) ucStream; + + pucReturn = FreeRTOS_get_tx_base( &xSocket ); + TEST_ASSERT_EQUAL_PTR( ( ( StreamBuffer_t * ) ucStream )->ucArray, pucReturn ); +} diff --git a/test/unit-test/FreeRTOS_Sockets/ut.cmake b/test/unit-test/FreeRTOS_Sockets/ut.cmake index 8803623e4..30a39aa6b 100644 --- a/test/unit-test/FreeRTOS_Sockets/ut.cmake +++ b/test/unit-test/FreeRTOS_Sockets/ut.cmake @@ -26,6 +26,20 @@ list(APPEND mock_list "${MODULE_ROOT_DIR}/test/unit-test/${project_name}/Sockets_list_macros.h" ) +# Without 'FreeRTOS_Sockets.h': +# +# 1 - FreeRTOS_Sockets_GenericAPI_utest (Not Run) +# 2 - FreeRTOS_Sockets_TCP_API_utest (Not Run) +# 3 - FreeRTOS_Sockets_UDP_API_utest (Not Run) +# 4 - FreeRTOS_Sockets_privates_utest (Not Run) +# +# With 'FreeRTOS_Sockets.h': +# +# 3 - FreeRTOS_Sockets_UDP_API_utest (Failed) +# +# FAIL:Function FreeRTOS_recvfrom. Called more times than expected. +# FAIL:Function FreeRTOS_sendto. Called more times than expected. + set(mock_include_list "") # list the directories your mocks need list(APPEND mock_include_list From 382ddb0795c78454b2ef925da367cec449c484e6 Mon Sep 17 00:00:00 2001 From: Hein Tibosch Date: Thu, 5 Oct 2023 13:56:32 +0800 Subject: [PATCH 2/2] Make use of FreeRTOS_inet_addr_quick() when applicable (#1032) --- source/include/FreeRTOS_Sockets.h | 12 ++++++------ .../NetworkInterface/WinPCap/NetworkInterface.c | 8 ++++---- .../NetworkInterface/linux/NetworkInterface.c | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/source/include/FreeRTOS_Sockets.h b/source/include/FreeRTOS_Sockets.h index 6ae8fdc86..b0c2be4ea 100644 --- a/source/include/FreeRTOS_Sockets.h +++ b/source/include/FreeRTOS_Sockets.h @@ -471,17 +471,17 @@ /* Converts an IP address expressed as four separate numeric octets into an * IP address expressed as a 32-bit number in network byte order */ #define FreeRTOS_inet_addr_quick( ucOctet0, ucOctet1, ucOctet2, ucOctet3 ) \ - ( ( ( ( uint32_t ) ( ucOctet3 ) ) << 24UL ) | \ - ( ( ( uint32_t ) ( ucOctet2 ) ) << 16UL ) | \ - ( ( ( uint32_t ) ( ucOctet1 ) ) << 8UL ) | \ + ( ( ( ( uint32_t ) ( ucOctet3 ) ) << 24 ) | \ + ( ( ( uint32_t ) ( ucOctet2 ) ) << 16 ) | \ + ( ( ( uint32_t ) ( ucOctet1 ) ) << 8 ) | \ ( ( uint32_t ) ( ucOctet0 ) ) ) #else /* ( ipconfigBYTE_ORDER == pdFREERTOS_BIG_ENDIAN ) */ #define FreeRTOS_inet_addr_quick( ucOctet0, ucOctet1, ucOctet2, ucOctet3 ) \ - ( ( ( ( uint32_t ) ( ucOctet0 ) ) << 24UL ) | \ - ( ( ( uint32_t ) ( ucOctet1 ) ) << 16UL ) | \ - ( ( ( uint32_t ) ( ucOctet2 ) ) << 8UL ) | \ + ( ( ( ( uint32_t ) ( ucOctet0 ) ) << 24 ) | \ + ( ( ( uint32_t ) ( ucOctet1 ) ) << 16 ) | \ + ( ( ( uint32_t ) ( ucOctet2 ) ) << 8 ) | \ ( ( uint32_t ) ( ucOctet3 ) ) ) #endif /* ( ipconfigBYTE_ORDER == pdFREERTOS_LITTLE_ENDIAN ) */ diff --git a/source/portable/NetworkInterface/WinPCap/NetworkInterface.c b/source/portable/NetworkInterface/WinPCap/NetworkInterface.c index f0a70bb7f..161c6e109 100644 --- a/source/portable/NetworkInterface/WinPCap/NetworkInterface.c +++ b/source/portable/NetworkInterface/WinPCap/NetworkInterface.c @@ -601,7 +601,7 @@ static void prvConfigureCaptureBehaviour( const NetworkInterface_t * pxInterface snprintf( cErrorBuffer, sizeof( cErrorBuffer ), "broadcast or multicast or ether host %x:%x:%x:%x:%x:%x", pucMAC[ 0 ], pucMAC[ 1 ], pucMAC[ 2 ], pucMAC[ 3 ], pucMAC[ 4 ], pucMAC[ 5 ] ); - ulNetMask = ( configNET_MASK3 << 24UL ) | ( configNET_MASK2 << 16UL ) | ( configNET_MASK1 << 8L ) | configNET_MASK0; + ulNetMask = FreeRTOS_inet_addr_quick( configNET_MASK0, configNET_MASK1, configNET_MASK2, configNET_MASK3 ); if( pcap_compile( pxOpenedInterfaceHandle, &xFilterCode, cErrorBuffer, 1, ulNetMask ) < 0 ) { @@ -697,7 +697,7 @@ DWORD WINAPI prvWinPcapRecvThread( void * pvParam ) { ( void ) pvParam; - /* THIS IS A WINDOWS THREAD - DO NOT ATTEMPT ANY FREERTOS CALLS OR TO PRINT + /* THIS IS A WINDOWS THREAD - DO NOT ATTEMPT ANY FREERTOS CALLS OR TO PRINT * OUT MESSAGES HERE. */ for( ; ; ) @@ -714,7 +714,7 @@ DWORD WINAPI prvWinPcapSendThread( void * pvParam ) static char cErrorMessage[ 1024 ]; const DWORD xMaxMSToWait = 1000; - /* THIS IS A WINDOWS THREAD - DO NOT ATTEMPT ANY FREERTOS CALLS OR TO PRINT + /* THIS IS A WINDOWS THREAD - DO NOT ATTEMPT ANY FREERTOS CALLS OR TO PRINT * OUT MESSAGES HERE. */ /* Remove compiler warnings about unused parameters. */ @@ -829,7 +829,7 @@ static void prvInterruptSimulatorTask( void * pvParameters ) BaseType_t xBounced = xPacketBouncedBack( pucPacketData ); /* Obtain a buffer into which the data can be placed. This - * is only an interrupt simulator, not a real interrupt, so it + * is only an interrupt simulator, not a real interrupt, so it * is ok to call the task level function here, but note that * some buffer implementations cannot be called from a real * interrupt. */ diff --git a/source/portable/NetworkInterface/linux/NetworkInterface.c b/source/portable/NetworkInterface/linux/NetworkInterface.c index 9ad95e006..ebf2bff26 100644 --- a/source/portable/NetworkInterface/linux/NetworkInterface.c +++ b/source/portable/NetworkInterface/linux/NetworkInterface.c @@ -589,10 +589,10 @@ static int prvOpenInterface( const char * pucName ) /*! * @brief Open the network interface. The number of the interface to be opened is - * set by the configNETWORK_INTERFACE_TO_USE constant in FreeRTOSConfig.h - * Calling this function will set the pxOpenedInterfaceHandle variable - * If, after calling this function, pxOpenedInterfaceHandle - * is equal to NULL, then the interface could not be opened. + * set by the configNETWORK_INTERFACE_TO_USE constant in FreeRTOSConfig.h + * Calling this function will set the pxOpenedInterfaceHandle variable + * If, after calling this function, pxOpenedInterfaceHandle + * is equal to NULL, then the interface could not be opened. * @param [in] pxAllNetworkInterfaces network interface list to choose from * @returns pdPASS on success or pdFAIL when something goes wrong */ @@ -717,7 +717,7 @@ static int prvConfigureCaptureBehaviour( void ) ipLOCAL_MAC_ADDRESS[ 5 ] ); FreeRTOS_debug_printf( ( "pcap filter to compile: %s\n", pcap_filter ) ); - ulNetMask = ( configNET_MASK3 << 24UL ) | ( configNET_MASK2 << 16UL ) | ( configNET_MASK1 << 8L ) | configNET_MASK0; + ulNetMask = FreeRTOS_inet_addr_quick( configNET_MASK0, configNET_MASK1, configNET_MASK2, configNET_MASK3 ); ret = pcap_compile( pxOpenedInterfaceHandle, &xFilterCode, @@ -960,7 +960,7 @@ static void prvInterruptSimulatorTask( void * pvParameters ) if( pxHeader->len <= ipTOTAL_ETHERNET_FRAME_SIZE ) { /* Obtain a buffer into which the data can be placed. This - * is only an interrupt simulator, not a real interrupt, so it + * is only an interrupt simulator, not a real interrupt, so it * is ok to call the task level function here, but note that * some buffer implementations cannot be called from a real * interrupt. */