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

Fix: make sure fprintf uses an unsigned marker when an unsigned value is given #1872

Merged
merged 1 commit into from Nov 15, 2021

Conversation

obones
Copy link
Contributor

@obones obones commented Nov 15, 2021

Fixes C6340 as reported in #1866

@merbanan merbanan merged commit 045b444 into merbanan:master Nov 15, 2021
@zuckschwerdt
Copy link
Collaborator

Not sure if works at intended, needs more research.
E.g. there is #define UINT16_MAX (65535) (or maybe #define UINT16_MAX 0xffff) which acording to literal rules will be int.
And in the printf's the (unsigned) shorts will be values will be coerced to integer according to promotion rules.

@merbanan
Copy link
Owner

Well the iterators could be changed to int. But int_t types should keep their size IIRC.

@obones
Copy link
Contributor Author

obones commented Nov 15, 2021

Under Visual Studio, it tells me that UINT16_MAX is defined like this in stdint.h

#define UINT16_MAX       0xffffui16

So in that instance, it clearly is a uint16_t

@zuckschwerdt
Copy link
Collaborator

Then let's start with the observation that all those instances were uint16_t. And as printf args they were not in there "native" size but all were coerced to int. This change now coerces to unsigned int. Doesn't matter anyway and both share the same conversion rank. But note that this likely changed the emitted code, not just tidied up some specifiers ;)

@merbanan
Copy link
Owner

But do we have different definitions of UINT16_MAX on different platforms ? That would be annoying.

@zuckschwerdt
Copy link
Collaborator

I don't think that matters, the literal would be at least uint16_t anyway and literal rules use the smallest type that fits the number. And any larger integral type will also fit. In the printf's we chose int, which can represent uint16_t. now we chose unsigned and the same applies ;) It should only matter to the compilers that we don't use hd, but hu and any higher rank should be fine.

@zuckschwerdt
Copy link
Collaborator

aside: it's easier for me to compile that changelog if commit messages follow https://triq.org/rtl_433/CONTRIBUTING.html#commit-messages
I.e. the verb is not followed by a colon, and the area-of-work would preferably be "minor:" here -- I can just filter those out as not relevant to the changelog :)

@obones
Copy link
Contributor Author

obones commented Nov 15, 2021

aside: it's easier for me to compile that changelog if commit messages follow https://triq.org/rtl_433/CONTRIBUTING.html#commit-messages

Bummer, I thought I did, but I now see that I missed the first part altogether. I'll keep that in mind for the next ones

@obones obones deleted the mismatch_on_sign branch October 3, 2023 08:36
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.

None yet

3 participants