Address MISRA Rule violations (#274)

* Use unsigned types/constants where needed.

* Address MISRA 21.15 violations in FreeRTOS_Sockets.c

* Address MISRA rule violations in code (primarily Rule 2.2)

* Inline had been disabled for Coverity builds, preventing
Coverity from correctly identifying dead code; this change
removes the disabling of inline during Coverity builds.
* Added an explanation for the inline suppression of Rule
11.4 in prvSocketValid().

Co-authored-by: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com>

3 files changed, 42 insertions, 50 deletions

diff --git a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c
index 0b89fdea6..1073a32b6 100644
--- a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c
+++ b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c
@@ -140,6 +140,15 @@ handled.  The value is chosen simply to be easy to spot when debugging. */
 had an invalid length. */
 #define ipINVALID_LENGTH			0x1234U
 
+/* Trace macros to aid in debugging, disabled if ipconfigHAS_PRINTF != 1 */
+#if ( ipconfigHAS_PRINTF == 1 )
+    #define DEBUG_DECLARE_TRACE_VARIABLE( type, var, init )    type var = ( init )
+    #define DEBUG_SET_TRACE_VARIABLE( var, value )  var = ( value )
+#else
+    #define DEBUG_DECLARE_TRACE_VARIABLE( type, var, init )
+    #define DEBUG_SET_TRACE_VARIABLE( var, value )
+#endif
+
 /*-----------------------------------------------------------*/
 
 /* Used in checksum calculation. */
@@ -163,10 +172,6 @@ static portINLINE ipDECL_CAST_PTR_FUNC_FOR_TYPE( NetworkBufferDescriptor_t )
 {
     return ( NetworkBufferDescriptor_t *)pvArgument;
 }
-static portINLINE ipDECL_CAST_CONST_PTR_FUNC_FOR_TYPE( NetworkBufferDescriptor_t )
-{
-    return ( const NetworkBufferDescriptor_t *) pvArgument;
-}
 
 /*-----------------------------------------------------------*/
 
@@ -2004,16 +2009,16 @@ uint8_t ucProtocol;
 	uint8_t ucProtocol;
 	uint16_t usLength;
 	uint16_t ucVersionHeaderLength;
-	BaseType_t xLocation = 0;
 	size_t uxMinimumLength;
 	BaseType_t xResult = pdFAIL;
+	DEBUG_DECLARE_TRACE_VARIABLE( BaseType_t, xLocation, 0 );
 
 		do
 		{
 			/* Check for minimum packet size: Ethernet header and an IP-header, 34 bytes */
 			if( uxBufferLength < sizeof( IPPacket_t ) )
 			{
-				xLocation = 1;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 1 );
 				break;
 			}
 
@@ -2027,7 +2032,7 @@ uint8_t ucProtocol;
 			if( ( ucVersionHeaderLength < ipIPV4_VERSION_HEADER_LENGTH_MIN ) ||
 				( ucVersionHeaderLength > ipIPV4_VERSION_HEADER_LENGTH_MAX ) )
 			{
-				xLocation = 2;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 2 );
 				break;
 			}
 			ucVersionHeaderLength = ( ucVersionHeaderLength & ( uint8_t ) 0x0FU ) << 2;
@@ -2036,7 +2041,7 @@ uint8_t ucProtocol;
 			/* Check if the complete IP-header is transferred. */
 			if( uxBufferLength < ( ipSIZE_OF_ETH_HEADER + uxIPHeaderLength ) )
 			{
-				xLocation = 3;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 3 );
 				break;
 			}
 			/* Check if the complete IP-header plus protocol data have been transferred: */
@@ -2044,7 +2049,7 @@ uint8_t ucProtocol;
 			usLength = FreeRTOS_ntohs( usLength );
 			if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
 			{
-				xLocation = 4;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 4 );
 				break;
 			}
 
@@ -2078,12 +2083,12 @@ uint8_t ucProtocol;
 			else
 			{
 				/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
-				xLocation = 5;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 5 );
 				break;
 			}
 			if( uxBufferLength < uxMinimumLength )
 			{
-				xLocation = 6;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 6 );
 				break;
 			}
 
@@ -2096,7 +2101,7 @@ uint8_t ucProtocol;
 				/* For incoming packets, the length is out of bound: either
 				too short or too long. For outgoing packets, there is a 
 				serious problem with the format/length. */
-				xLocation = 7;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 7 );
 				break;
 			}
 			xResult = pdPASS;
@@ -2104,12 +2109,8 @@ uint8_t ucProtocol;
 
 		if( xResult != pdPASS )
 		{
+			/* NOP if ipconfigHAS_PRINTF != 1 */
 			FreeRTOS_printf( ( "xCheckSizeFields: location %ld\n", xLocation ) );
-			
-			/* If FreeRTOS_printf is not defined, not using xLocation will be a violation of MISRA
-			 * rule 2.2 as the value assigned to xLocation will not be used. The below statement uses
-			 * the variable without modifying the logic of the source. */
-			( void ) xLocation;
 		}
 
 		return xResult;
@@ -2130,9 +2131,7 @@ uint8_t ucProtocol;
 #endif
 uint16_t usLength;
 uint16_t ucVersionHeaderLength;
-
-
-BaseType_t location = 0;
+DEBUG_DECLARE_TRACE_VARIABLE( BaseType_t, xLocation, 0 );
 
 	/* Introduce a do-while loop to allow use of break statements.
 	 * Note: MISRA prohibits use of 'goto', thus replaced with breaks. */
@@ -2142,7 +2141,7 @@ BaseType_t location = 0;
 		if( uxBufferLength < sizeof( IPPacket_t ) )
 		{
 			usChecksum = ipINVALID_LENGTH;
-			location = 1;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 1 );
 			break;
 		}
 
@@ -2159,7 +2158,7 @@ BaseType_t location = 0;
 		if( uxBufferLength < ( sizeof( IPPacket_t ) + ( uxIPHeaderLength - ipSIZE_OF_IPv4_HEADER ) ) )
 		{
 			usChecksum = ipINVALID_LENGTH;
-			location = 2;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 2 );
 			break;
 		}
 		usLength = pxIPPacket->xIPHeader.usLength;
@@ -2167,7 +2166,7 @@ BaseType_t location = 0;
 		if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
 		{
 			usChecksum = ipINVALID_LENGTH;
-			location = 3;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 3 );
 			break;
 		}
 
@@ -2187,7 +2186,7 @@ BaseType_t location = 0;
 			if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_UDP_HEADER ) )
 			{
 				usChecksum = ipINVALID_LENGTH;
-				location = 4;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 4 );
 				break;
 			}
 
@@ -2203,7 +2202,7 @@ BaseType_t location = 0;
 			if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_TCP_HEADER ) )
 			{
 				usChecksum = ipINVALID_LENGTH;
-				location = 5;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 5 );
 				break;
 			}
 
@@ -2220,7 +2219,7 @@ BaseType_t location = 0;
 			if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_ICMP_HEADER ) )
 			{
 				usChecksum = ipINVALID_LENGTH;
-				location = 6;
+				DEBUG_SET_TRACE_VARIABLE( xLocation, 6 );
 				break;
 			}
 
@@ -2242,7 +2241,7 @@ BaseType_t location = 0;
 		{
 			/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
 			usChecksum = ipUNHANDLED_PROTOCOL;
-			location = 7;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 7 );
 			break;
 		}
 
@@ -2280,7 +2279,7 @@ BaseType_t location = 0;
 				usChecksum = ipCORRECT_CRC;
 			}
 			#endif
-			location = 8;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 8 );
 			break;
 		}
 		else
@@ -2307,7 +2306,7 @@ BaseType_t location = 0;
 			For outgoing packets, there is a serious problem with the
 			format/length */
 			usChecksum = ipINVALID_LENGTH;
-			location = 9;
+			DEBUG_SET_TRACE_VARIABLE( xLocation, 9 );
 			break;
 		}
 		if( ucProtocol <= ( uint8_t ) ipPROTOCOL_IGMP )
@@ -2383,12 +2382,8 @@ BaseType_t location = 0;
 	if( ( usChecksum == ipUNHANDLED_PROTOCOL ) || 
 		( usChecksum == ipINVALID_LENGTH ) )
 	{
-		FreeRTOS_printf( ( "CRC error: %04x location %ld\n", usChecksum, location ) );
-		
-		/* If FreeRTOS_printf is not defined, not using 'location' will be a violation of MISRA
-		 * rule 2.2 as the value assigned to 'location' will not be used. The below statement uses
-		 * the variable without modifying the logic of the source. */
-		( void ) location;
+		/* NOP if ipconfigHAS_PRINTF != 0 */
+		FreeRTOS_printf( ( "CRC error: %04x location %ld\n", usChecksum, xLocation ) );
 	}
 
 	return usChecksum;
diff --git a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_IP.h b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_IP.h
index b0cbf25d9..440f22abe 100644
--- a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_IP.h
+++ b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_IP.h
@@ -38,20 +38,6 @@ extern "C" {
 #include "FreeRTOSIPConfigDefaults.h"
 #include "IPTraceMacroDefaults.h"
 
-#ifdef __COVERITY__
-	/* Coverity static checks don't like inlined functions.
-	As it is up to the users to allow inlining, don't let
-	let Coverity know about it. */
-
-	#ifdef portINLINE
-		/* The usage of #undef violates the rule. */
-		#undef portINLINE
-
-	#endif
-
-	#define	portINLINE
-#endif
-
 /* Some constants defining the sizes of several parts of a packet.
 These defines come before inlucding the configuration header files. */
 /* The size of the Ethernet header is 14, meaning that 802.1Q VLAN tags
diff --git a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_Sockets.h b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_Sockets.h
index c9580fed1..7f5fcf91a 100644
--- a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_Sockets.h
+++ b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/include/FreeRTOS_Sockets.h
@@ -207,8 +207,19 @@ typedef struct xSOCKET const * ConstSocket_t;
 
 static portINLINE unsigned int prvSocketValid( Socket_t xSocket )
 {
+    unsigned int lReturnValue = pdFALSE;
+    /*
+     * There are two values which can indicate an invalid socket:
+     * FREERTOS_INVALID_SOCKET and NULL.  In order to compare against
+     * both values, the code cannot be compliant with rule 11.4,
+     * hence the Coverity suppression statement below.
+     */
     /* coverity[misra_c_2012_rule_11_4_violation] */
-    return ( ( xSocket != FREERTOS_INVALID_SOCKET ) && ( xSocket != NULL ) );
+    if( ( xSocket != FREERTOS_INVALID_SOCKET ) && ( xSocket != NULL ) )
+    {
+	    lReturnValue = pdTRUE;
+    }
+    return lReturnValue;
 }
 
 #if( ipconfigSUPPORT_SELECT_FUNCTION == 1 )