Skip to content

Misra fix or suppress remaining violations #529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Aug 8, 2022
Merged

Misra fix or suppress remaining violations #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
35d787d
Fix Remaning misra issues
alfred2g Jul 28, 2022
a76d481
Suppress rule 8.6
alfred2g Jul 28, 2022
18dfde3
Fix/Suppress more misra violations
alfred2g Jul 29, 2022
e0c458c
Style: for formatting
alfred2g Jul 29, 2022
68d597c
Style: fix formatting
alfred2g Jul 29, 2022
9541e0d
Style: fix spelling
alfred2g Jul 29, 2022
13c58cb
Fix Rule 11.1
alfred2g Aug 1, 2022
57caf76
Fix undeteced suppressions
alfred2g Aug 1, 2022
a9dd58d
Enable 32 bits
alfred2g Aug 1, 2022
d2f5158
Fix more misra leftover violations
alfred2g Aug 1, 2022
1567226
Add justification for a missed violation
alfred2g Aug 1, 2022
2d2e745
Fix comment for rule 8.13
alfred2g Aug 1, 2022
40c82c9
Fix comment
alfred2g Aug 3, 2022
e3156cc
fix misra comments
alfred2g Aug 3, 2022
81e90d8
Update MISRA.md
alfred2g Aug 3, 2022
0ece037
Update source/FreeRTOS_Sockets.c
alfred2g Aug 3, 2022
6f6d630
Update source/portable/BufferManagement/BufferAllocation_2.c
alfred2g Aug 3, 2022
fb24ef8
Update MISRA.md
alfred2g Aug 3, 2022
46705be
Update MISRA.md
alfred2g Aug 3, 2022
561419c
Suppress Rule 8.9
alfred2g Aug 3, 2022
464ff7b
Fix build error
alfred2g Aug 3, 2022
c945cde
fix build error
alfred2g Aug 3, 2022
5ab12ef
Fix coverity supression bugs
alfred2g Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 51 additions & 10 deletions MISRA.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,35 @@ _Ref 8.9.1_
order of execution, some variables have file scope definitions rather
than function scope.

#### Rule 8.13
_Ref 8.13.1_

- MISRA C-2012 Rule 8.13 Parameter passed is never used, should be declared as
Copy link
Contributor

@paulbartell paulbartell Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be re-worded to be more generic. Something like:

MISRA C-2012 Rule 8.13 encourages the use of const-qualified types whenever possible. However, the FreeRTOS API requires that all task entry functions conform to the same function signature (TaskFunction_t) which includes a single void * pointer argument. Due to this restriction, it is not possible to use a const qualifier for the entry point function to any FreeRTOS task.

Copy link
Member

@AniruddhaKanhere AniruddhaKanhere Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was fixed with this commit: 81e90d8

Copy link
Contributor

@paulbartell paulbartell Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: It's too specific to the particular instance. We're likely to run into this exact violation quite frequently.

Copy link
Contributor Author

@alfred2g alfred2g Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is ok to be specific might be even desirable, we do support multiple References for specific cases

const. The argument passed to the `prvIPTask` function is left unused which is
considered as the variable not being used and thus warranting the use of `const`.
However, the FreeRTOS-kernel function `xTaskCreate` expects a function signature
of type `void vSomeFunction( void * pvArgs )`. To satisfy that requirement, the
function signature of `prvIPTask` does not have a `const` qualifier in the
parameter signature.

#### Rule 10.5
_Ref 10.5.1_

- MISRA C-2012 Rule 10.5 Converting from an unsigned to an enum type. The
operation is safe to perform in that case, as we are using a generic API
to send and receive data, in that case the exact data sent it is received

#### Rule 11.1
_Ref 11.1.1_

- MISRA C-2012 Rule 11.1 Converting from a void pointer to a function pointer.
The `FreeRTOS_setsockopt` API allows users to configure sockets by setting
various options. In order to do so, the function must accept one parameter
which, based on the option value, can be casted to the corresponding socket
field. To that end, that parameter is of `void *` type to accommodate all values.
The caller of the API is responsible for providing correct function pointer to the
API. Thus, this violation can be safely suppressed.

#### Rule 11.3
_Ref 11.3.1_

Copy link
Contributor

@paulbartell paulbartell Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about for 64-bit targets that do not allow 32-bit aligned access?

Copy link
Contributor Author

@alfred2g alfred2g Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently do not support any hardware with that feature/drawback the only 64 bit we currently support is RiscV and they do support unaligned access (if i am not mistaken)
when we support more 64bit hardware we will have to add code to cover that case.

Copy link
Contributor

@htibosch htibosch Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulbartell wrote:

What about for 64-bit targets that do not allow 32-bit aligned access?

A 64-bit target that do not allow 32-bit aligned access? I am not sure what you mean here.

@Alfred2 wrote:

We currently do not support any hardware with that feature/drawback the only 64 bit we currently support is RiscV

As for FreeRTOS+TCP, we also support the 64-bit UltraScale platform with a Cortex-A53.

I have never seen alignment problems with the casting of types.

Copy link
Contributor

@paulbartell paulbartell Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Cortex-A53 is only strict about alignment when configured such in the SCTLR. It's pretty common to configure it to allow unaligned access.

In contrast, a Cortex-M0 (ARMv6-M) will always fail unaligned accesses.. but is 4-byte aligned.

SPARC and MIPS are both example of a 64bit architectures that do not support unaligned access... So de-referencing a 32-bit aligned pointer as a 64bit type could cause a fault at runtime.

In the case of FR+TCP, we're unlikely to run into this problem in particular.

Expand Down Expand Up @@ -107,16 +136,6 @@ _Ref 14.3.1_
- MISRA C-2012 Rule 14.3 False positive as the value might be changed
depending on the conditionally compiled code

#### Rule 21.6
_Ref 21.6.1_

- MISRA C-2012 Rule 21.6 warns about the use of standard library input/output
functions as they might have implementation defined or undefined
behaviour. The function `snprintf` is used to insert information in a
logging string. This is only used in a utility function which aids in
debugging and is not part of the 'core' code governing the
functionality of the TCP/IP stack.

#### Rule 17.2
_Ref 17.2.1_

Expand All @@ -128,10 +147,32 @@ _Ref 17.2.1_
have a secondary child socket thereby limiting the number of recursive
calls to one.

#### Rule 20.5
_Ref 20.5.1_

- MISRA C-2012 Rule 20.5 warns against the use of #undef.
FreeRTOS-Plus-TCP allows its users to set some configuration macros
to modify the behavior/performance of the library according to their
needs. However, the macros values must be within certain bounds.
To achieve that, if the macro values lie outside of the bounds, they
are undefined using `#undef` before being redefined to a proper
value.

#### Rule 20.10
_Ref 20.10.1_

- MISRA C-2012 Rule 20.10 warns against the use of ## concatination operator.
However, in this case, it must be used to support compile time
assertions in case the preprocessor does not suppport sizeof. This
operation (assert) has no runtime execution.

#### Rule 21.6
_Ref 21.6.1_

- MISRA C-2012 Rule 21.6 warns about the use of standard library input/output
Copy link
Contributor

@paulbartell paulbartell Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This justification should mention how we prevent misuse of printf-style functions. Mentioning "-Wformat-security" and/or specific Wformat options that are used when building unit tests would be helpful here.

functions as they might have implementation defined or undefined
behaviour. The function `snprintf` is used to insert information in a
logging string. This is only used in a utility function which aids in
debugging and is not part of the 'core' code governing the
functionality of the TCP/IP stack.

34 changes: 23 additions & 11 deletions source/FreeRTOS_IP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ static void prvProcessIPEventsAndTimers( void );
* from the network hardware drivers and tasks that are using sockets. It also
* maintains a set of protocol timers.
*/
static void prvIPTask( const void * pvParameters );
static void prvIPTask( void * pvParameters );

/*
* Called when new data is available from the network interface.
Expand Down Expand Up @@ -229,7 +229,11 @@ static BaseType_t xIPTaskInitialised = pdFALSE;
*
* @param[in] pvParameters: Not used.
*/
static void prvIPTask( const void * pvParameters )

/* MISRA Ref 8.13.1 [Not decorating a pointer to const parameter with const] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-813 */
/* coverity[misra_c_2012_rule_8_13_violation] */
static void prvIPTask( void * pvParameters )
{
/* Just to prevent compiler warnings about unused parameters. */
( void ) pvParameters;
Expand Down Expand Up @@ -390,9 +394,11 @@ static void prvProcessIPEventsAndTimers( void )
eDHCPState_t eState;

/* MISRA Ref 11.6.1 [DHCP events and conversion to void] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-116 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-116 */
/* coverity[misra_c_2012_rule_11_6_violation] */
uxState = ( uintptr_t ) xReceivedEvent.pvData;
/* MISRA Ref 10.5.1 [DHCP events Enum] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-105 */
/* coverity[misra_c_2012_rule_10_5_violation] */
eState = ( eDHCPState_t ) uxState;

Expand Down Expand Up @@ -708,6 +714,7 @@ void * FreeRTOS_GetUDPPayloadBuffer( size_t uxRequestedSizeBytes,
* @return pdPASS if the task was successfully created and added to a ready
* list, otherwise an error code defined in the file projdefs.h
*/
/* coverity[single_use] */
BaseType_t FreeRTOS_IPInit( const uint8_t ucIPAddress[ ipIP_ADDRESS_LENGTH_BYTES ],
const uint8_t ucNetMask[ ipIP_ADDRESS_LENGTH_BYTES ],
const uint8_t ucGatewayAddress[ ipIP_ADDRESS_LENGTH_BYTES ],
Expand Down Expand Up @@ -1171,7 +1178,7 @@ eFrameProcessingResult_t eConsiderFrameForProcessing( const uint8_t * const pucE
/* Map the buffer onto Ethernet Header struct for easy access to fields. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxEthernetHeader = ( ( const EthernetHeader_t * ) pucEthernetBuffer );

Expand Down Expand Up @@ -1245,7 +1252,7 @@ static void prvProcessEthernetPacket( NetworkBufferDescriptor_t * const pxNetwor
/* Map the buffer onto the Ethernet Header struct for easy access to the fields. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxEthernetHeader = ( ( const EthernetHeader_t * ) pxNetworkBuffer->pucEthernetBuffer );

Expand All @@ -1263,7 +1270,7 @@ static void prvProcessEthernetPacket( NetworkBufferDescriptor_t * const pxNetwor
if( pxNetworkBuffer->xDataLength >= sizeof( ARPPacket_t ) )
{
/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
eReturned = eARPProcessPacket( ( ( ARPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer ) );
}
Expand All @@ -1280,7 +1287,7 @@ static void prvProcessEthernetPacket( NetworkBufferDescriptor_t * const pxNetwor
if( pxNetworkBuffer->xDataLength >= sizeof( IPPacket_t ) )
{
/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
eReturned = prvProcessIPPacket( ( ( IPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer );
}
Expand Down Expand Up @@ -1542,7 +1549,7 @@ static eFrameProcessingResult_t prvAllowIPPacket( const IPPacket_t * const pxIPP
/* pxProtPack will point to the offset were the protocols begin. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxProtPack = ( ( ProtocolPacket_t * ) &( pxNetworkBuffer->pucEthernetBuffer[ uxHeaderLength - ipSIZE_OF_IPv4_HEADER ] ) );

Expand Down Expand Up @@ -1610,6 +1617,10 @@ static eFrameProcessingResult_t prvProcessIPPacket( IPPacket_t * pxIPPacket,
/* Check if the IP headers are acceptable and if it has our destination. */
eReturn = prvAllowIPPacket( pxIPPacket, pxNetworkBuffer, uxHeaderLength );

/* MISRA Ref 14.3.1 [Configuration dependent invariant] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */
/* coverity[misra_c_2012_rule_14_3_violation] */
/* coverity[cond_const] */
if( eReturn == eProcessBuffer )
{
/* Are there IP-options. */
Expand Down Expand Up @@ -1647,8 +1658,9 @@ static eFrameProcessingResult_t prvProcessIPPacket( IPPacket_t * pxIPPacket,
}

/* MISRA Ref 14.3.1 [Configuration dependent invariant] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */
/* coverity[misra_c_2012_rule_14_3_violation] */
/* coverity[const] */
if( eReturn != eReleaseBuffer )
{
/* Add the IP and MAC addresses to the ARP table if they are not
Expand Down Expand Up @@ -1838,7 +1850,7 @@ static eFrameProcessingResult_t prvProcessIPPacket( IPPacket_t * pxIPPacket,
* fields of the IP packet. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxIPPacket = ( ( const IPPacket_t * ) pucEthernetBuffer );

Expand Down Expand Up @@ -1991,7 +2003,7 @@ void vReturnEthernetFrame( NetworkBufferDescriptor_t * pxNetworkBuffer,
/* Map the Buffer to Ethernet Header struct for easy access to fields. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxEthernetHeader = ( ( EthernetHeader_t * ) pxNetworkBuffer->pucEthernetBuffer );

Expand Down
3 changes: 2 additions & 1 deletion source/FreeRTOS_IP_Utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
/* MISRA Ref 8.9.1 [File scoped variables] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-89 */
/* coverity[misra_c_2012_rule_8_9_violation] */
/* coverity[single_use] */
static BaseType_t xCallEventHook = pdFALSE;
#endif

Expand Down Expand Up @@ -974,10 +975,10 @@ uint16_t usGenerateChecksum( uint16_t usSum,
/* coverity[value_overwrite] */
xSum.u32 = ( uint32_t ) xSum.u16[ 0 ] + xSum.u16[ 1 ];

/* coverity[value_overwrite] */
/* MISRA Ref 2.2.1 [Unions and dead code] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-22 */
/* coverity[misra_c_2012_rule_2_2_violation] */
/* coverity[value_overwrite] */
xSum.u32 = ( uint32_t ) xSum.u16[ 0 ] + xSum.u16[ 1 ];

if( ( uxAlignBits & 1U ) != 0U )
Expand Down
Loading