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 various warnings #187

Merged
merged 3 commits into from Feb 4, 2020
Merged

fix various warnings #187

merged 3 commits into from Feb 4, 2020

Conversation

Adsun701
Copy link
Contributor

@Adsun701 Adsun701 commented Jan 9, 2020

This suppresses some harmless warnings given when compiling on MinGW-w64 due to differences in formatting. It also fixes some "dllimport" warnings and corrects a conditional block in cmake/FindPOPT.cmake.

Fixes #184.

@dbaarda
Copy link
Member

dbaarda commented Jan 9, 2020

So this fixes #184 by suppressing warnings about using "%I64" format strings on compilers that support the C99 "%uz" formatting we probably should be using.

We are using old windows "%I64" format strings in this case because when cross-compiling our CMakeLists.txt assumes we don't support "%zu" because we can't actively check for it. I have a feeling that it's probably better to assume that any compiler capable of cross-compiling is also C99 compliant and supports "%zu".

So I'm not sure about the "-Wno-format" part of this, but thanks heaps for the rest... I'm not sure how we missed correctly exporting the trace stuff.

Instead of suppressing warnings for all platforms about format strings with `-Wno-format` because minGW complains about using `%I64`, assume that any compiler advanced enough to cross-compile is also C99 compliant and supports `%zu`.
@dbaarda
Copy link
Member

dbaarda commented Jan 10, 2020

I've change the CMakeLists.txt to do what I think is a better solution for the format warnings. However, I don't have easy access to test that this actually works as I think it should.

Can you check if this is OK? If you are happy with this I'll merge it.

@Adsun701
Copy link
Contributor Author

There are still warnings regarding "ISO C not supporting the I64 modifier". In addition, there are warnings regarding unknown conversion type character ‘z’ in format.

@dbaarda
Copy link
Member

dbaarda commented Jan 11, 2020

Ugh, so it really doesn't support "%uz" but still complains about "%I64", which we are also using for FMT_LONG. I guess it's using the target platform's implementation of printf.

I'll need to have a dig around to figure out the best solution. It would be nice if we could find a format specifier for these! Hat it doesn't complain about. Failing that we can silence the warnings for WIN32 only.

@dbaarda
Copy link
Member

dbaarda commented Jan 20, 2020

The warnings it's giving about using %I64 for FMT_LONG comes from the #define here;

#define FMT_LONG "%"PRIdMAX

Which is basically using PRIdMAX defined in inttypes.h, which is part of the C99 standard. So it's warning about one of the C99 compatible headers provided for the WIN32 platform. It seems that using %I64 and %I is right for that platform, and the only thing we can do about the warnings for that platform is to turn them off.

I'm going to change this to turn of the warnings only for WIN32, and I'm going to simplify it to assume C99 compliance for other platforms.

This reverts the last commit and instead turns of warnings for WIN32 only.
@dbaarda
Copy link
Member

dbaarda commented Feb 2, 2020

I finally got a chance to have another attempt at this. Can you please test it and make sure it works as intended?

@Adsun701
Copy link
Contributor Author

Adsun701 commented Feb 4, 2020

@dbaarda It works great!

@dbaarda dbaarda merged commit e76f689 into librsync:master Feb 4, 2020
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.

Multiple "ISO C does not support the ‘I64’ ms_printf length modifier" warnings with MinGW-w64
2 participants