Skip to content

[Coverity CID :196640]Integer handling issues in /arch/x86/core/thread.c #14816

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.

Sign up for GitHub

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

Jump to bottom
Closed
aasthagr opened this issue Mar 22, 2019 · 0 comments
Closed

[Coverity CID :196640]Integer handling issues in /arch/x86/core/thread.c #14816

aasthagr opened this issue Mar 22, 2019 · 0 comments
Assignees
@andyross @andrewboie
Labels
area: Architectures bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: high High impact/importance bug

Comments

@aasthagr
Copy link
Collaborator

Static code scan issues seen in File: /arch/x86/core/thread.c
Category: Integer handling issues
Function: z_new_thread
Component: Architectures
CID: 196640
Please fix or provide comments to square it off in coverity in the link: https://scan9.coverity.com/reports.htm#v32951/p12996
@aasthagr aasthagr added area: Architectures bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix labels Mar 22, 2019
@galak galak added the priority: high High impact/importance bug label Mar 22, 2019
@andyross andyross mentioned this issue Mar 22, 2019
Merged
andyross pushed a commit to andyross/zephyr that referenced this issue Mar 22, 2019
kernel/sched: Rework prio validity assertion
46c9c59 
This is throwing errors in static analysis, complaining that comparing
that a prior is higher and lower is impossible.  That is wrong per my
eyes (I swear I think it might be cueing off the names of the
functions, which invert "higher" and "lower" to match our reversed
priority numbers).

But frankly this was never a very readable macro to begin with.
Refactor to put the bounds into the term, so the static analyzer can
prove it locally, and add a build assertion to catch any errors (there
are none currently) where the low<->high priority range is invalid.

Long term, we should probably remove this macro, it doesn't provide
much value.  But removing it in response to a static analysis failure
is... not very responsible as a development practice.

Fixes zephyrproject-rtos#14816
Fixes zephyrproject-rtos#14820

Signed-off-by: Andy Ross <[email protected]>
galak pushed a commit that referenced this issue Mar 23, 2019
@galak
kernel/sched: Rework prio validity assertion
e59d196 
This is throwing errors in static analysis, complaining that comparing
that a prior is higher and lower is impossible.  That is wrong per my
eyes (I swear I think it might be cueing off the names of the
functions, which invert "higher" and "lower" to match our reversed
priority numbers).

But frankly this was never a very readable macro to begin with.
Refactor to put the bounds into the term, so the static analyzer can
prove it locally, and add a build assertion to catch any errors (there
are none currently) where the low<->high priority range is invalid.

Long term, we should probably remove this macro, it doesn't provide
much value.  But removing it in response to a static analysis failure
is... not very responsible as a development practice.

Fixes #14816
Fixes #14820

Signed-off-by: Andy Ross <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andyross andyross

@andrewboie andrewboie

Labels
area: Architectures bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: high High impact/importance bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@andyross @andrewboie @galak @aasthagr