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

Misc fixes found by static code analysis #178

Merged
merged 8 commits into from
May 24, 2023

Conversation

ralphlange
Copy link
Member

A set of fixes for different issues (few major, many minor) that were found by cppcheck/sonar at ITER.

@AppVeyorBot
Copy link

Build asyn 1.0.224 failed (commit fcedd3dafa by @ralphlange)

@ralphlange ralphlange requested a review from MarkRivers April 28, 2023 22:13
Copy link
Member

@MarkRivers MarkRivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphlange this looks good thanks.

My only comment is that changing %u to %lu for size_t arguments is still not correct on 64-bit Windows, right? There a size_t is 64-bit but a long is 32-bit. Changing is to *llu won't be correct on 32-bit machines since a size_t is 32-bit and a long long is 64-bit? Should we cast the argument instead?

@AppVeyorBot
Copy link

Build asyn 1.0.224 failed (commit fcedd3dafa by @ralphlange)

@norumwe12
Copy link
Contributor

norumwe12 commented Apr 29, 2023 via email

@AppVeyorBot
Copy link

Build asyn 1.0.224 completed (commit fcedd3dafa by @ralphlange)

@MarkRivers
Copy link
Member

I presume this is an issue because there are platforms that don’t support the ‘z’ modifier for size_t (“%zu”) nor the “PRI” string literals?
Perhaps some EPICS OS-dependent header could provide the PRIxxx literals for such machines?

This has been discussed in this tech-talk thread. Apparently of the supported architectures only vxWorks does not support %z.
https://epics.anl.gov/core-talk/2022/msg00321.php

@ralphlange
Copy link
Member Author

True, I didn't consider that.
Guess the only waterproof option is to cast the argument to the widest in-the-wild type that we need to support. Anything else would either fail or be insufficient on some architectures. Only if that widest type is not supported by all compilers, we'd be in trouble.

@MarkRivers
Copy link
Member

I thing cast to (long long) and %llu would work on all?

@ralphlange
Copy link
Member Author

Worth a try.
Better than %z that would fail hard on VxWorks, while the other mismatches would just generate warnings.

use %llu and cast argument to (unsigned long long)
@AppVeyorBot
Copy link

Build asyn 1.0.226 completed (commit 87e2e1944f by @ralphlange)

@MarkRivers MarkRivers merged commit b691d1f into epics-modules:master May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants