Bug in HAL I2C code - V1.11.2 HAL_I2C_Mem_Write() function

[Robmar]

Can you see the glaring bug, or is it?  I left a clue *** some fail returns do not unlock HAL

HAL_StatusTypeDef HAL_I2C_Mem_Write(I2C_HandleTypeDef *hi2c, uint16_t DevAddress, uint16_t MemAddress,
                                    uint16_t MemAddSize, uint8_t *pData, uint16_t Size, uint32_t Timeout)
{
  uint32_t tickstart;

  /* Check the parameters */
  assert_param(IS_I2C_MEMADD_SIZE(MemAddSize));

  if (hi2c->State == HAL_I2C_STATE_READY)
  {
    if ((pData == NULL) || (Size == 0U))
    {
      hi2c->ErrorCode = HAL_I2C_ERROR_INVALID_PARAM;
      return  HAL_ERROR;
    }

    /* Process Locked ***/
    __HAL_LOCK(hi2c);

    /* Init tickstart for timeout management*/
    tickstart = HAL_GetTick();

    if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BUSY, SET, I2C_TIMEOUT_BUSY, tickstart) != HAL_OK)
    {
      return HAL_ERROR;
    }

    hi2c->State     = HAL_I2C_STATE_BUSY_TX;
    hi2c->Mode      = HAL_I2C_MODE_MEM;
    hi2c->ErrorCode = HAL_I2C_ERROR_NONE;

    /* Prepare transfer parameters */
    hi2c->pBuffPtr  = pData;
    hi2c->XferCount = Size;
    hi2c->XferISR   = NULL;

    /* Send Slave Address and Memory Address */
    if (I2C_RequestMemoryWrite(hi2c, DevAddress, MemAddress, MemAddSize, Timeout, tickstart) != HAL_OK)
    {
      /* Process Unlocked */
      __HAL_UNLOCK(hi2c);
      return HAL_ERROR;
    }
...

[Robmar]

There is also another fail here in this function definition, one arm has a return, the other does not.  Sloppy really as the compiler even reports the issue "Return has value in function returning void".

  #define __HAL_LOCK(__HANDLE__)                                           \
                                do{                                        \
                                    if((__HANDLE__)->Lock == HAL_LOCKED)   \
                                    {                                      \
                                       return HAL_BUSY;                    \
                                    }                                      \
                                    else                                   \
                                    {                                      \
                                       (__HANDLE__)->Lock = HAL_LOCKED;    \
                                    }                                      \
                                  }while (0)

[Bob S]

I admit up front I am no fan of the "return hidden in a macro" coding style of the __HAL_LOCK() macro.

That said, you should not see that compiler warning for the __HAL_LOCK() call in HAL_I2C_Mem_Write() as that function DOES have a return value.  So where DID you see that compiler warning?

Which CPU?  I presume the "V1.11.2" refer to the Cube library version?

[Robmar]

The defined function has no return specification so the compiler assumes its void, then one arm of the "if" returns a value, of course its going to give a warning!  I´m using the latest version of CubeIDE and the warning is on the tool tip when hovering over the use of the defined function __HAL_LOCK and in the compiler warnings.

MCU is H743, Repository is the latest V1.11.2 at -> STM32Cube\Repository\STM32Cube_FW_H7_V1.11.2\Drivers\STM32H7xx_HAL_Driver\Src

[SofLit]

Escalated internally to be fixed.

Internal ticket number for tracking 189599.