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

[Coverity CID :219649] Operands don't affect result in kernel/sem.c #33038

Closed
zephyrbot opened this issue Mar 7, 2021 · 5 comments
Closed

[Coverity CID :219649] Operands don't affect result in kernel/sem.c #33038

zephyrbot opened this issue Mar 7, 2021 · 5 comments
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug

Comments

@zephyrbot
Copy link
Collaborator

Static code scan issues found in file:

https://github.com/zephyrproject-rtos/zephyr/tree/bd97359a5338b2542d19011b6d6aa1d8d1b9cc3f/kernel/sem.c#L68

Category: Integer handling issues
Function: z_impl_k_sem_init
Component: Kernel
CID: 219649

Details:

CHECKIF(limit == 0U || initial_count > limit) {

62     int z_impl_k_sem_init(struct k_sem *sem, unsigned int initial_count,
63                   unsigned int limit)
64     {
65         /*
66          * Limit cannot be zero and count cannot be greater than limit
67          */
>>>     CID 219649:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
>>>     "limit > 4294967295U /* 2147483647 * 2U + 1U */" is always false regardless of the values of its operands. This occurs as the logical second operand of "||".
68         CHECKIF(limit == 0U || limit > K_SEM_MAX_LIMIT || initial_count > limit) {
69             return -EINVAL;
70         }
71    
72         sem->count = initial_count;
73         sem->limit = limit;

Please fix or provide comments in coverity using the link:

https://scan9.coverity.com/reports.htm#v32951/p12996.

Note: This issue was created automatically. Priority was set based on classification
of the file affected and the impact field in coverity. Assignees were set using the CODEOWNERS file.

@zephyrbot zephyrbot added bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug labels Mar 7, 2021
@dcpleung
Copy link
Member

dcpleung commented Mar 7, 2021

@jharris-intel is limit > K_SEM_MAX_LIMIT needed in this case? I see K_SEM_MAX_LIMIT is #define to be UINT_MAX.

@jharris-intel
Copy link
Contributor

What's the recommended approach when a limiting constant could be UINT_MAX in some, but not all, configurations?

That constant was added because I've got a WIP PR that adds an implementation whose limit is INT_MAX.

@dcpleung
Copy link
Member

dcpleung commented Mar 8, 2021

I assume we can ignore it as intentional in Coverity? @ceolin what do you think?

@ceolin
Copy link
Member

ceolin commented Mar 8, 2021

I assume we can ignore it as intentional in Coverity? @ceolin what do you think?

Yep, we have others similar cases. This just need to be triaged as intentional

@dcpleung
Copy link
Member

dcpleung commented Mar 8, 2021

Done.

@dcpleung dcpleung closed this as completed Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

6 participants