stm32h7xx_hal_driver icon indicating copy to clipboard operation
stm32h7xx_hal_driver copied to clipboard

Most pointer inputs arguemnts to HAL Drivers functions do not treat the data they are pointing to as a constant data.

Open rxa1031 opened this issue 4 years ago • 4 comments

Hi @ALABSTM , @ASELSTM ,

In order to prevent unwanted modifications to output data to be sent out on any port, I usually prefer to typecast the used pointer and the variable to constant. U am observing that I cannot pass such constant data pointing pointers and constant data to any HAL Driver function:

Example:

	#pragma pack(1)

		typedef struct
		{
			uint16_t	u16MajorNumber;
			uint16_t	u16MinorNumber;
			uint16_t	u16PatchNumber;
			uint8_t	ucReleaseType;
		} goPSCIsAliveResponse;

		const goPSCIsAliveResponse	oPSCIsAliveResponse =
		{
			.u16MajorNumber	= VERSION_MAJOR,
			.u16MinorNumber	= VERSION_MINOR,
			.u16PatchNumber	= VERSION_PATCH,
			.ucReleaseType	= VERSION_EXTRA[0]
		};

	#pragma pack()
		Status = HAL_FDCAN_AddMessageToTxFifoQ(phfdcan, &oPSCIsAliveResponseTxHeader, &oPSCIsAliveResponse);

The Keil compiler throws:

../Src/main.c(778): error: embedded position-independent lowering of oPSCIsAliveResponse made it non-const, other translation units will access it incorrectly [-Werror,-Wembedded-pi]
                const goPSCIsAliveResponse      oPSCIsAliveResponse =
                                                ^
../Src/main.c(1331): error: incompatible pointer types passing 'const goPSCIsAliveResponse *' to parameter of type 'uint8_t *' (aka 'unsigned char *') [-Werror,-Wincompatible-pointer-types]
                Status = HAL_FDCAN_AddMessageToTxFifoQ(phfdcan, &oPSCIsAliveResponseTxHeader, &oPSCIsAliveResponse);
                                                                                              ^~~~~~~~~~~~~~~~~~~~
../Drivers/STM32H7xx_HAL_Driver/Inc\stm32h7xx_hal_fdcan.h(2021): note: passing argument to parameter 'pTxData' here
HAL_StatusTypeDef HAL_FDCAN_AddMessageToTxFifoQ(FDCAN_HandleTypeDef *hfdcan, FDCAN_TxHeaderTypeDef *pTxHeader, uint8_t *pTxData);
                                                                                                                        ^

I therefore request ST team to modify the HAL Driver APIs and function calls so as to allow directly passing:

  1. Pointer to constant data
  2. Constant Pointer to constant data.

Example: It will be helpful if the current function declaration

HAL_StatusTypeDef HAL_FDCAN_AddMessageToTxFifoQ(FDCAN_HandleTypeDef *hfdcan, FDCAN_TxHeaderTypeDef *pTxHeader, uint8_t *pTxData)

is changed to:

HAL_StatusTypeDef HAL_FDCAN_AddMessageToTxFifoQ(FDCAN_HandleTypeDef * const hfdcan, const FDCAN_TxHeaderTypeDef * const pTxHeader, const uint8_t * const pTxData)

Please share your views.

Thanks, Rajeev

rxa1031 avatar May 02 '20 14:05 rxa1031

If I change the call to below:

		Status = HAL_FDCAN_AddMessageToTxFifoQ(phfdcan, &oPSCIsAliveResponseTxHeader, (uint8_t *)&oPSCIsAliveResponse);

wherein I am typecasting the oPSCIsAliveResponse when passing it to the function, the Keil throws below Error:

../Src/main.c(778): error: embedded position-independent lowering of oPSCIsAliveResponse made it non-const, other translation units will access it incorrectly [-Werror,-Wembedded-pi]
                const goPSCIsAliveResponse      oPSCIsAliveResponse =

I would have loved to call the function as shown below:

// Variables used in my code:
	const FDCAN_TxHeaderTypeDef oPSCIsAliveResponseTxHeader;
	const goPSCIsAliveResponse	oPSCIsAliveResponse;

// Arguments that I would pass to function:
FDCAN_HandleTypeDef * const phfdcan = (FDCAN_HandleTypeDef * const) &hfdcan1;
const FDCAN_TxHeaderTypeDef * const pTxHeader = (const FDCAN_TxHeaderTypeDef * cons)&oPSCIsAliveResponseTxHeader;
const uint8_t *const pTxData = (const uint8_t * const)&oPSCIsAliveResponse;

// Function Call
Status = HAL_FDCAN_AddMessageToTxFifoQ(phfdcan, pTxHeader , pTxData);

rxa1031 avatar May 02 '20 14:05 rxa1031

Hi @rxa1031,

Thank you for this detailed report. This issue has also been previously reported by other users (link).

This is one of the main points in the tasks queue of our development teams. We cannot share a date regarding its deployment and publication. We count on your patience and comprehension.

Thank you again for this report. Take care and stay safe.

With regards,

ALABSTM avatar May 04 '20 12:05 ALABSTM

Hello @RKOUSTM

Will there be a new release shortly with fix available for the reported issue?

Thanks, Rajeev

rxa1031 avatar Apr 14 '21 16:04 rxa1031

Hello @ALABSTM I've just found this issue and so I'm not opening a duplicate. I'd like saying that a fix should be necessary soon, otherwise there are a lot of warning when transmit functions (as an example HAL_SPI_Transmit) are called under the hood from platform_write implemented as requested by STMems_Standard_C_drivers. As an example https://github.com/STMicroelectronics/STMems_Standard_C_drivers/blob/928331ce8eaa61ed9b5df91c632d6733bb4bf52f/iis3dwb_STdC/examples/iis3dwb_self_test.c#L117-L118

A dirty workaround implemented in some source code is to cast-away the const-ness of the input parameter, but this is definitively bad practise and should be avoided. Moreover modern C++ compilers would threat this as warnings, and this could be a potential major issue in project that needs some kind of certifications.

escherstair avatar Jul 09 '21 13:07 escherstair

ST Internal Reference: 98091

ALABSTM avatar May 11 '23 09:05 ALABSTM

Hi everyone,

The deployment of the const keyword is being deployed slowly but surely as said before. Below the example of the HAL and the LL I²C drivers, fixed in commit https://github.com/STMicroelectronics/stm32h7xx_hal_driver/commit/ab5b9a231a4866ea98f621127219b903af8a220c. In the meanwhile, please allow me to close this issue. This topic will be tracked through our internal trackers till it is fully deployed.

https://github.com/STMicroelectronics/stm32h7xx_hal_driver/blob/8d27047ce061c1d164b8a1f9a3d7ea71a11c2649/Inc/stm32h7xx_hal_i2c.h#L712-L714

With regards,

ALABSTM avatar May 11 '23 09:05 ALABSTM