misrac: Checking the returned value of a non-void function #18344
Conversation
MISRA-C rule 17.7 says that the return value of a non-void must be checked. For memcpy/memset there is no error handling or checking we can do with the return value, so it is just casting it to void to explicitly ignore while adhering to MISRA-C. This patch was generated using the coccinelle script ignore_return.cocci Signed-off-by: Flavio Ceolin <[email protected]>
ceolin
requested review from
aescolar,
alexanderwachter,
andrewboie,
andyross,
avisconti,
carlescufi,
cvinayak,
dcpleung,
erwango,
finikorg,
galak,
ioannisg,
jakub-uC,
jfischer-no,
jhedberg,
joerchan,
jukkar,
Laczen,
loicpoulain,
Mani-Sadhasivam,
MaureenHelm,
mike-scott,
nashif,
nordic-krch,
nvlsianpu,
pabigot,
pfalcon,
rlubos and
rveerama1
as code owners
|
17.7 is a required rule, which allows deviations. A reasonable deviation is that when the return value is a constant that cannot affect control flow, as it is with |
I know that, I'm imagining if we start create deviation for this simple case we'll end up with deviations for almost all rules, honestly. People will be much more sensitive to other cases. Is that what we want, deviate from every single rule ? |
|
Regardless of how this rule is applied, I would really appreciate it if Coccinelle fixes were applied in two stages: first a single commit that adds the SmPL script (possibly in scripts/coccinelle/misra or .../codingrules, or...). Then a commit that applies the script. Not having access to the scripts that perform the rewrite makes it harder for people to verify they aren't introducing new violations, let alone integrate checks into any sort of QA process. |
|
@ceolin Do you intend to continuing pursing this PR to merge? |
github-actions
bot
added
area: ARM
github-actions
bot
added
platform: STM32
| @@ -260,18 +260,18 @@ static int bt_ipm_send(struct net_buf *buf) | |||
| buf->len); | |||
| k_sem_take(&acl_data_ack, K_FOREVER); | |||
| net_buf_push_u8(buf, HCI_ACL); | |||
| memcpy((void *) | |||
| (void)memcpy((void *) | |||
| &((TL_AclDataPacket_t *)HciAclDataBuffer)->AclDataSerial, | |||
There was a problem hiding this comment.
This line should be re-indented.
| memcpy(&attr, &static_svc->attrs[i], | ||
| sizeof(attr)); | ||
| (void)memcpy(&attr, &static_svc->attrs[i], | ||
| sizeof(attr)); |
There was a problem hiding this comment.
AFAICT the alignment is still off. How much effort should we put into this if we cannot do it with a script? It's almost 250 files and the PR has been up for a while.
|
Will close this outdated patch and re-open a new one later. |
MISRA-C rule 17.7 says that the return value of a non-void must be
checked. For memcpy/memset there is no error handling or checking we
can do with the return value, so it is just casting it to void to
explicitly ignore while adhering to MISRA-C.
This patch was generated using the coccinelle script
ignore_return.cocci
Signed-off-by: Flavio Ceolin [email protected]