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

Possible negative char to isspace - static analysis warning #1208

Closed
skliper opened this issue Mar 4, 2021 · 1 comment · Fixed by #1230 or #1243
Closed

Possible negative char to isspace - static analysis warning #1208

skliper opened this issue Mar 4, 2021 · 1 comment · Fixed by #1230 or #1243
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Mar 4, 2021

Is your feature request related to a problem? Please describe.
In theory a negative value could reach this code through public APIs, which would lead to undefined isspace behavior as it gets converted to int.

while (StringLen > 0 && isspace((int)Buffer[StringLen-1]))

Describe the solution you'd like
Recommended practice is to cast to unsigned char, such that the conversion to int results in defined behavior.

Describe alternatives you've considered
Could adjust all the parameters involved to unsigned char, but probably not worth it.

Additional context
Static analysis warning

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 7.0.0 milestone Mar 4, 2021
@jphickey
Copy link
Contributor

jphickey commented Mar 4, 2021

I went back to the docs on this function, and it says this:

The  c  argument is an int, the value of which the application shall ensure is a character representable as an
unsigned char or equal to the value of the macro EOF. If the argument has any other value, the behavior is un‐
defined.

.... suggests that we actually should cast to unsigned char rather than int when using any of the ctype checks.

skliper added a commit to skliper/cFE that referenced this issue Mar 16, 2021
@skliper skliper self-assigned this Mar 16, 2021
skliper added a commit to skliper/cFE that referenced this issue Mar 17, 2021
astrogeco added a commit that referenced this issue Mar 18, 2021
Fix #1208, Cast isspace input to unsigned char
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants