Skip to content
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.

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

FLASH_OB_GetRDP has 32-bit return type but HAL_FLASHEx_OBErase implicitly converts it to 8-bits #6

Open
jacobq opened this issue Jan 19, 2024 · 3 comments
Assignees
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request.

Comments

@jacobq
Copy link

jacobq commented Jan 19, 2024

Describe the set-up

  • Hardware agnostic (encountered while compiling program for STM32F072)
  • STM32CubeIDE v1.14.0 (build 19471_20231121_1200 (UTC))

Describe the bug
Compiling stm32f0xx_hal_flash_ex.c results in (conversion related) warnings at build-time like this:

...
arm-none-eabi-gcc "../Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_rcc_ex.c" -mcpu=cortex-m0 -std=gnu11 -g3 -DDEBUG -DUSE_HAL_DRIVER -DSTM32F072xB -c -I../Core/Inc -I../Drivers/STM32F0xx_HAL_Driver/Inc -I../Drivers/STM32F0xx_HAL_Driver/Inc/Legacy -I../Drivers/CMSIS/Device/ST/STM32F0xx/Include -I../Drivers/CMSIS/Include -Og -ffunction-sections -fdata-sections -Wall -Wconversion -fstack-usage -fcyclomatic-complexity -MMD -MP -MF"Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_rcc_ex.d" -MT"Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_rcc_ex.o" --specs=nano.specs -mfloat-abi=soft -mthumb -o "Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_rcc_ex.o"
../Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c: In function 'HAL_FLASHEx_OBErase':
../Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c:317:12: warning: conversion from 'uint32_t' {aka 'long unsigned int'} to 'uint8_t' {aka 'unsigned char'} may change value [-Wconversion]
  317 |   rdptmp = FLASH_OB_GetRDP();
      |            ^~~~~~~~~~~~~~~
../Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c: In function 'HAL_FLASHEx_OBGetConfig':
../Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c:449:23: warning: conversion from 'uint32_t' {aka 'long unsigned int'} to 'uint8_t' {aka 'unsigned char'} may change value [-Wconversion]
  449 |   pOBInit->RDPLevel = FLASH_OB_GetRDP();
      |                       ^~~~~~~~~~~~~~~
...
13:52:02 Build Finished. 0 errors, 2 warnings. (took 4s.966ms

How To Reproduce

  1. Application agnostic.
  2. Suspect is HAL (stm32f0xx_hal_flash_ex.c)
  3. No need to make any calls to reproduce -- problem is warning at compile time.
  4. Can reproduce by compiling stm32f0xx_hal_flash_ex.c with conversion warnings enabled.

Additional context
My suggested solution is to simply change the return type from uint32_t to uint8_t since result must be one of OB_RDP_LEVEL_0, OB_RDP_LEVEL_1, or OB_RDP_LEVEL_2

diff --git a/Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c b/Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c
index 7ea91b9..34068c7 100644
--- a/Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c
+++ b/Drivers/STM32F0xx_HAL_Driver/Src/stm32f0xx_hal_flash_ex.c
@@ -102,7 +102,7 @@ static HAL_StatusTypeDef FLASH_OB_RDP_LevelConfig(uint8_t ReadProtectLevel);
 static HAL_StatusTypeDef FLASH_OB_UserConfig(uint8_t UserConfig);
 static HAL_StatusTypeDef FLASH_OB_ProgramData(uint32_t Address, uint8_t Data);
 static uint32_t          FLASH_OB_GetWRP(void);
-static uint32_t          FLASH_OB_GetRDP(void);
+static uint8_t           FLASH_OB_GetRDP(void);
 static uint8_t           FLASH_OB_GetUser(void);

 /**
@@ -899,7 +899,7 @@ static uint32_t FLASH_OB_GetWRP(void)
   *            @arg @ref OB_RDP_LEVEL_1 Read protection of the memory
   *            @arg @ref OB_RDP_LEVEL_2 Full chip protection
   */
-static uint32_t FLASH_OB_GetRDP(void)
+static uint8_t FLASH_OB_GetRDP(void)
 {
   uint32_t tmp_reg;
@ALABSTM ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. labels Jan 22, 2024
@TOUNSTM
Copy link
Contributor

TOUNSTM commented Jan 23, 2024

See also stm32f1xx_hal_driver#7

@jacobq
Copy link
Author

jacobq commented Jan 24, 2024

If changing the signature is too much to ask, perhaps an explicit cast to suppress the warning would be OK?
https://github.com/STMicroelectronics/stm32f0xx_hal_driver/blob/5fd31c52559a593dd9038c4abb4b85faeac728c6/Src/stm32f0xx_hal_flash_ex.c#L317

rdptmp = (uint8_t)FLASH_OB_GetRDP();

Then again, these are static ("private implementation details") so I don't see how fixing this would negatively impact any user code.

@TOUNSTM
Copy link
Contributor

TOUNSTM commented Jan 24, 2024

Hello @jacobq,

Thank you for your report. We confirm the appearance of warnings by activating -Wconversion warnings. This problem has been reported to our development teams for further analysis. We'll get back to you as soon as we have any updates.

With Regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request.
Projects
Development

No branches or pull requests

3 participants