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 MinGW format attribute #125

Closed
wants to merge 1 commit into from
Closed

fix MinGW format attribute #125

wants to merge 1 commit into from

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Mar 5, 2024

Setting -D__USE_MINGW_ANSI_STDIO=1 is wrong and should not be used. MinGW internally uses a macro to select between gnu_printf and printf. Just use that instead of using a wrong format under clang backends.

Setting -D__USE_MINGW_ANSI_STDIO=1 is wrong and should not be used. MinGW
internally uses a macro to select between gnu_printf and printf. Just use
that instead of using a wrong format under clang backends.

Signed-off-by: Rosen Penev <[email protected]>
@dgibson
Copy link
Owner

dgibson commented Mar 5, 2024

@elmarco you originally added that -D as part of the meson build. Does this change seem reasonable to you?

@elmarco
Copy link
Contributor

elmarco commented Mar 5, 2024

The rationale is from commit 588a29f. I don't know whether this has changed.

Using __MINGW_PRINTF_FORMAT makes the situation even more complex imho, as we end up with 3 format variants.

@neheb what are you solving? clang doesn't support __USE_MINGW_ANSI_STDIO ?

@neheb
Copy link
Contributor Author

neheb commented Mar 5, 2024

clang doesn’t support gnu_printf. That define is used to solve format warnings when ms_printf is used. It’s not the correct solution.

@elmarco
Copy link
Contributor

elmarco commented Mar 5, 2024

as long as it compiles and run the tests, I guess it's fine. @dgibson

@dgibson
Copy link
Owner

dgibson commented Mar 6, 2024

Well, the tests aren't so useful, since I'm not set up to do Windows builds. Oh well, I don't see anything obviously wrong, so applying.

@dgibson dgibson closed this Mar 6, 2024
@dgibson
Copy link
Owner

dgibson commented Mar 6, 2024

@neheb if you have the chance and ability to add a github job for windows builds of dtc, that would be fantastic. I don't know enough about either gitlab actions or windows to do so.

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.

3 participants