drivers: usb: stm32: HAL return value handling #97283
Conversation
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
djiatsaf-st,
erwango,
gautierg-st,
jfischer-no,
marwaiehm-st,
mathieuchopstm and
tmon-nordic
There was a problem hiding this comment.
Changes in this file are no longer accepted AFAIK.
There was a problem hiding this comment.
Most (all?) of these functions cannot fail so I don't see any value in adding error checking.
There was a problem hiding this comment.
Then why do they have a return value. It's weird regarding Zephyr guideline "If a function returns error information, then that error information shall be tested".
That said I'm fine dropping these changes. After seeing a few mixup of HAL return values and standard errno value, I wanted to clean then hence this change series. I expect not functional change with these patches but better HAL integration example for future contributions.
There was a problem hiding this comment.
Maybe an assertion on return value would be enough.
Alternatively I think at least an inline comment should state that it's safe to discard the return value.
There was a problem hiding this comment.
Then why do they have a return value. It's weird regarding Zephyr guideline "If a function returns error information, then that error information shall be tested".
Something something HAL guidelines. Alas...
Maybe an assertion on return value would be enough. Alternatively I think at least an inline comment should state that it's safe to discard the return value.
__ASSERT_NO_MSG(hal_ret == HAL_OK); checks would be fine by me.
There was a problem hiding this comment.
Ok, i'll change to assertion.
etienne-lms
force-pushed
the
stm32-hal-ret-usb
branch
2 times, most recently
from
de57d85 to
46eea7f
Compare
Add missing test of some HAL functions return value;. Signed-off-by: Etienne Carriere <[email protected]>
etienne-lms
force-pushed
the
stm32-hal-ret-usb
branch
from
46eea7f to
5ca3380
Compare
|
Add missing test of some HAL functions return value.