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

Correct Format Specifiers for message macros (ie ferror()m dmainfo()) printf and scanf usage going forward. #3181

Closed
davids5 opened this issue Mar 25, 2021 · 0 comments

Comments

@davids5
Copy link
Contributor

davids5 commented Mar 25, 2021

From some discussions.

I have seen a lot of fixes for this class of error using different approaches. @patacongo mentioned the conical form as you have used here in. I was trying to discern the correct pattern and if it is divided on a common OS and arch driver boundary or are we just not being consistent and should ask for changes on PRs.

To that end, do you feel we should add an explicit #include <inttypes.h> to these files?

I don't really know the answer to that; on one hand if we know that, e.g., uint32_t needs "%lu" for a particular arch, then we could use that, but then it will likely be copied-and-pasted into code for other arches. Using the PRIu32 et al for these (IMO) makes the code a little harder to grok but seems to be the safest approach as the code will DTRT for any arch. Doing nothing and leaving the format specifiers as they are now is raising a lot of compiler warnings since the build system is set up that way since some months ago. I fixed this file today because my build started throwing warnings as soon as I enabled DMA in Kconfig.

The correct pattern and rules to apply (Thank you @gustavonihei !!)

I believe this is not related to having a pattern across the OS, is more like standards-related.
If you use the native types (int, long, short...) than the recommended is to use the conversion specifiers from the ' fprintf' documentation:
https://pubs.opengroup.org/onlinepubs/009695399/functions/fprintf.html

If you use the fixed-width integer types from stdint.h (uint32_t, int32_t, int8_t...), then it is recommended to use the conversion specifiers from inttypes.h. By using this approach you abstract implementation details and the code will be portable across different architectures.
https://en.cppreference.com/w/c/types/integer

So, if you keep using %ld for int32_t on an armv7-a machine, it is correct from the compiler's point of view. But it is not correct regarding abstraction, since you are leaking machine implementation details to your application/driver.

Editors Note: Add an explicit #include <inttypes.h> to files using the PRxxx macros.

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

No branches or pull requests

2 participants