mirror of
https://github.com/FreeRTOS/FreeRTOS-Kernel.git
synced 2025-10-16 09:47:44 -04:00
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>
This commit is contained in:
parent
50dc98a5a6
commit
c997d887e0
3 changed files with 42 additions and 50 deletions
|
@ -140,6 +140,15 @@ handled. The value is chosen simply to be easy to spot when debugging. */
|
||||||
had an invalid length. */
|
had an invalid length. */
|
||||||
#define ipINVALID_LENGTH 0x1234U
|
#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. */
|
/* Used in checksum calculation. */
|
||||||
|
@ -163,10 +172,6 @@ static portINLINE ipDECL_CAST_PTR_FUNC_FOR_TYPE( NetworkBufferDescriptor_t )
|
||||||
{
|
{
|
||||||
return ( NetworkBufferDescriptor_t *)pvArgument;
|
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;
|
uint8_t ucProtocol;
|
||||||
uint16_t usLength;
|
uint16_t usLength;
|
||||||
uint16_t ucVersionHeaderLength;
|
uint16_t ucVersionHeaderLength;
|
||||||
BaseType_t xLocation = 0;
|
|
||||||
size_t uxMinimumLength;
|
size_t uxMinimumLength;
|
||||||
BaseType_t xResult = pdFAIL;
|
BaseType_t xResult = pdFAIL;
|
||||||
|
DEBUG_DECLARE_TRACE_VARIABLE( BaseType_t, xLocation, 0 );
|
||||||
|
|
||||||
do
|
do
|
||||||
{
|
{
|
||||||
/* Check for minimum packet size: Ethernet header and an IP-header, 34 bytes */
|
/* Check for minimum packet size: Ethernet header and an IP-header, 34 bytes */
|
||||||
if( uxBufferLength < sizeof( IPPacket_t ) )
|
if( uxBufferLength < sizeof( IPPacket_t ) )
|
||||||
{
|
{
|
||||||
xLocation = 1;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 1 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2027,7 +2032,7 @@ uint8_t ucProtocol;
|
||||||
if( ( ucVersionHeaderLength < ipIPV4_VERSION_HEADER_LENGTH_MIN ) ||
|
if( ( ucVersionHeaderLength < ipIPV4_VERSION_HEADER_LENGTH_MIN ) ||
|
||||||
( ucVersionHeaderLength > ipIPV4_VERSION_HEADER_LENGTH_MAX ) )
|
( ucVersionHeaderLength > ipIPV4_VERSION_HEADER_LENGTH_MAX ) )
|
||||||
{
|
{
|
||||||
xLocation = 2;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 2 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
ucVersionHeaderLength = ( ucVersionHeaderLength & ( uint8_t ) 0x0FU ) << 2;
|
ucVersionHeaderLength = ( ucVersionHeaderLength & ( uint8_t ) 0x0FU ) << 2;
|
||||||
|
@ -2036,7 +2041,7 @@ uint8_t ucProtocol;
|
||||||
/* Check if the complete IP-header is transferred. */
|
/* Check if the complete IP-header is transferred. */
|
||||||
if( uxBufferLength < ( ipSIZE_OF_ETH_HEADER + uxIPHeaderLength ) )
|
if( uxBufferLength < ( ipSIZE_OF_ETH_HEADER + uxIPHeaderLength ) )
|
||||||
{
|
{
|
||||||
xLocation = 3;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 3 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
/* Check if the complete IP-header plus protocol data have been transferred: */
|
/* Check if the complete IP-header plus protocol data have been transferred: */
|
||||||
|
@ -2044,7 +2049,7 @@ uint8_t ucProtocol;
|
||||||
usLength = FreeRTOS_ntohs( usLength );
|
usLength = FreeRTOS_ntohs( usLength );
|
||||||
if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
|
if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
|
||||||
{
|
{
|
||||||
xLocation = 4;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 4 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2078,12 +2083,12 @@ uint8_t ucProtocol;
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
|
/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
|
||||||
xLocation = 5;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 5 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if( uxBufferLength < uxMinimumLength )
|
if( uxBufferLength < uxMinimumLength )
|
||||||
{
|
{
|
||||||
xLocation = 6;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 6 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2096,7 +2101,7 @@ uint8_t ucProtocol;
|
||||||
/* For incoming packets, the length is out of bound: either
|
/* For incoming packets, the length is out of bound: either
|
||||||
too short or too long. For outgoing packets, there is a
|
too short or too long. For outgoing packets, there is a
|
||||||
serious problem with the format/length. */
|
serious problem with the format/length. */
|
||||||
xLocation = 7;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 7 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
xResult = pdPASS;
|
xResult = pdPASS;
|
||||||
|
@ -2104,12 +2109,8 @@ uint8_t ucProtocol;
|
||||||
|
|
||||||
if( xResult != pdPASS )
|
if( xResult != pdPASS )
|
||||||
{
|
{
|
||||||
|
/* NOP if ipconfigHAS_PRINTF != 1 */
|
||||||
FreeRTOS_printf( ( "xCheckSizeFields: location %ld\n", xLocation ) );
|
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;
|
return xResult;
|
||||||
|
@ -2130,9 +2131,7 @@ uint8_t ucProtocol;
|
||||||
#endif
|
#endif
|
||||||
uint16_t usLength;
|
uint16_t usLength;
|
||||||
uint16_t ucVersionHeaderLength;
|
uint16_t ucVersionHeaderLength;
|
||||||
|
DEBUG_DECLARE_TRACE_VARIABLE( BaseType_t, xLocation, 0 );
|
||||||
|
|
||||||
BaseType_t location = 0;
|
|
||||||
|
|
||||||
/* Introduce a do-while loop to allow use of break statements.
|
/* Introduce a do-while loop to allow use of break statements.
|
||||||
* Note: MISRA prohibits use of 'goto', thus replaced with breaks. */
|
* Note: MISRA prohibits use of 'goto', thus replaced with breaks. */
|
||||||
|
@ -2142,7 +2141,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < sizeof( IPPacket_t ) )
|
if( uxBufferLength < sizeof( IPPacket_t ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 1;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 1 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2159,7 +2158,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < ( sizeof( IPPacket_t ) + ( uxIPHeaderLength - ipSIZE_OF_IPv4_HEADER ) ) )
|
if( uxBufferLength < ( sizeof( IPPacket_t ) + ( uxIPHeaderLength - ipSIZE_OF_IPv4_HEADER ) ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 2;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 2 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
usLength = pxIPPacket->xIPHeader.usLength;
|
usLength = pxIPPacket->xIPHeader.usLength;
|
||||||
|
@ -2167,7 +2166,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
|
if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ( size_t ) usLength ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 3;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 3 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2187,7 +2186,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_UDP_HEADER ) )
|
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_UDP_HEADER ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 4;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 4 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2203,7 +2202,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_TCP_HEADER ) )
|
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_TCP_HEADER ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 5;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 5 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2220,7 +2219,7 @@ BaseType_t location = 0;
|
||||||
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_ICMP_HEADER ) )
|
if( uxBufferLength < ( uxIPHeaderLength + ipSIZE_OF_ETH_HEADER + ipSIZE_OF_ICMP_HEADER ) )
|
||||||
{
|
{
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 6;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 6 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2242,7 +2241,7 @@ BaseType_t location = 0;
|
||||||
{
|
{
|
||||||
/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
|
/* Unhandled protocol, other than ICMP, IGMP, UDP, or TCP. */
|
||||||
usChecksum = ipUNHANDLED_PROTOCOL;
|
usChecksum = ipUNHANDLED_PROTOCOL;
|
||||||
location = 7;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 7 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2280,7 +2279,7 @@ BaseType_t location = 0;
|
||||||
usChecksum = ipCORRECT_CRC;
|
usChecksum = ipCORRECT_CRC;
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
location = 8;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 8 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
|
@ -2307,7 +2306,7 @@ BaseType_t location = 0;
|
||||||
For outgoing packets, there is a serious problem with the
|
For outgoing packets, there is a serious problem with the
|
||||||
format/length */
|
format/length */
|
||||||
usChecksum = ipINVALID_LENGTH;
|
usChecksum = ipINVALID_LENGTH;
|
||||||
location = 9;
|
DEBUG_SET_TRACE_VARIABLE( xLocation, 9 );
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if( ucProtocol <= ( uint8_t ) ipPROTOCOL_IGMP )
|
if( ucProtocol <= ( uint8_t ) ipPROTOCOL_IGMP )
|
||||||
|
@ -2383,12 +2382,8 @@ BaseType_t location = 0;
|
||||||
if( ( usChecksum == ipUNHANDLED_PROTOCOL ) ||
|
if( ( usChecksum == ipUNHANDLED_PROTOCOL ) ||
|
||||||
( usChecksum == ipINVALID_LENGTH ) )
|
( usChecksum == ipINVALID_LENGTH ) )
|
||||||
{
|
{
|
||||||
FreeRTOS_printf( ( "CRC error: %04x location %ld\n", usChecksum, location ) );
|
/* NOP if ipconfigHAS_PRINTF != 0 */
|
||||||
|
FreeRTOS_printf( ( "CRC error: %04x location %ld\n", usChecksum, xLocation ) );
|
||||||
/* 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;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return usChecksum;
|
return usChecksum;
|
||||||
|
|
|
@ -38,20 +38,6 @@ extern "C" {
|
||||||
#include "FreeRTOSIPConfigDefaults.h"
|
#include "FreeRTOSIPConfigDefaults.h"
|
||||||
#include "IPTraceMacroDefaults.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.
|
/* Some constants defining the sizes of several parts of a packet.
|
||||||
These defines come before inlucding the configuration header files. */
|
These defines come before inlucding the configuration header files. */
|
||||||
/* The size of the Ethernet header is 14, meaning that 802.1Q VLAN tags
|
/* The size of the Ethernet header is 14, meaning that 802.1Q VLAN tags
|
||||||
|
|
|
@ -207,8 +207,19 @@ typedef struct xSOCKET const * ConstSocket_t;
|
||||||
|
|
||||||
static portINLINE unsigned int prvSocketValid( Socket_t xSocket )
|
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] */
|
/* 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 )
|
#if( ipconfigSUPPORT_SELECT_FUNCTION == 1 )
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue