Good afternoon
According to the project requirements, two keys were added to the compiler settings for stricter project verification:
-Wsign-conversion - Checking for the presence of implicit signed / unsigned conversions in the code
-Wconversion - Checking for Implicit Conversions in Code
After installing these keys, a large number of warnings were found in the project. Some of them were related to files "arm_cmsis". Description of the problem here: CMSIS_5_issues. To solve the problem, I simply updated СMSIS to version 5.1.1 (in SDK the version was 5.1.0)
But the rest of the problems are related to transformations in SDK files.
For example:
// file app_util.h
/**@brief Function for decoding a uint16 value.
*
* @param[in] p_encoded_data Buffer where the encoded data is stored.
*
* @return Decoded value.
*/
static __INLINE uint16_t uint16_decode(const uint8_t * p_encoded_data)
{
return ( (((uint16_t)((uint8_t *)p_encoded_data)[0])) |
(((uint16_t)((uint8_t *)p_encoded_data)[1]) << 8 ));
}
Warning: conversion from 'int' to 'uint16_t' {aka 'short unsigned int'} may change value [-Wconversion]
can be simplified to the expression:
return (uint16_t) ( p_encoded_data[0] | ( (uint16_t)(p_encoded_data[1]) << 8 ));
This is just one example.
A big request, if possible, check the SDK with these compilation keys, as the warning appears in unexpected places, for example, in files "nrf_sdh.c", "nrf_sdh_ble.c"
Edit:
file port_cmsis.c
BaseType_t xPortStartScheduler( void )
{
/* configMAX_SYSCALL_INTERRUPT_PRIORITY must not be set to 0.
See http://www.FreeRTOS.org/RTOS-Cortex-M3-M4.html */
configASSERT( configMAX_SYSCALL_INTERRUPT_PRIORITY );
/* This port is designed for nRF52, this is Cortex-M4 r0p1. */
configASSERT( SCB->CPUID == portCORTEX_M4_r0p1_ID );
#if ( configASSERT_DEFINED == 1 )
{
volatile uint32_t ulOriginalPriority;
volatile uint8_t * const pucFirstUserPriorityRegister = &NVIC->IP[0];
volatile uint8_t ucMaxPriorityValue;
/* Determine the maximum priority from which ISR safe FreeRTOS API
functions can be called. ISR safe functions are those that end in
"FromISR". FreeRTOS maintains separate thread and ISR API functions to
ensure interrupt entry is as fast and simple as possible.
Save the interrupt priority value that is about to be clobbered. */
ulOriginalPriority = *pucFirstUserPriorityRegister;
/* Determine the number of priority bits available. First write to all
possible bits. */
*pucFirstUserPriorityRegister = portMAX_8_BIT_VALUE;
/* Read the value back to see how many bits stuck. */
ucMaxPriorityValue = *pucFirstUserPriorityRegister;
/* Use the same mask on the maximum system call priority. */
ucMaxSysCallPriority = configMAX_SYSCALL_INTERRUPT_PRIORITY & ucMaxPriorityValue;
/* Calculate the maximum acceptable priority group value for the number
of bits read back. */
ulMaxPRIGROUPValue = SCB_AIRCR_PRIGROUP_Msk >> SCB_AIRCR_PRIGROUP_Pos;
while ( ( ucMaxPriorityValue & portTOP_BIT_OF_BYTE ) == portTOP_BIT_OF_BYTE )
{
ulMaxPRIGROUPValue--;
ucMaxPriorityValue <<= ( uint8_t ) 0x01;
}
/* Remove any bits that are more than actually existing. */
ulMaxPRIGROUPValue &= SCB_AIRCR_PRIGROUP_Msk >> SCB_AIRCR_PRIGROUP_Pos;
/* Restore the clobbered interrupt priority register to its original
value. */
*pucFirstUserPriorityRegister = ulOriginalPriority;
}
#endif /* conifgASSERT_DEFINED */
...
}
a potentially dangerous place, because a 32-bit word is written to a pointer to an 8-bit variable.
/* Restore the clobbered interrupt priority register to its original value. */
*pucFirstUserPriorityRegister = ulOriginalPriority;
Warning: conversion from 'uint32_t' {aka 'volatile unsigned int'} to 'uint8_t' {aka 'volatile unsigned char'} may change value [-Wconversion]
file: ble_gattc.h
a potentially dangerous place,
if ((p_next - p_first) / (sizeof(uint16_t) + value_len) < p_gattc_evt->params.char_val_by_uuid_read_rsp.count) // w1
{
p_iter->handle = (uint16_t)p_next[1] << 8 | p_next[0]; // w2
p_iter->p_value = p_next + sizeof(uint16_t);
return NRF_SUCCESS;
}
w1: conversion to 'unsigned int' from 'int' may change the sign of the result [-Wsign-conversion]
w2: conversion from 'int' to 'uint16_t' {aka 'short unsigned int'} may change value [-Wconversion]
In my opinion, there are not enough brackets here. In addition, in the comparison condition, there may be a situation of comparing signed and unsigned numbers due to implicit conversion.
Maybe I'm wrong in this particular place, but such an error easily leads to a situation, for example, when -3 is greater than 3.
I hope these warnings are false positive, but I wish they weren't.
Thank you for understanding
