C file should not include <cinttypes>, it is a C++ header.

[spetrunia]

Include <inttypes.h> instead.

[spetrunia]

Background:
The issue is caused by a recent patch in RocksDB:
d68f9f4
the patch did a bulk change:

-#include <inttypes.h>
+#include <cinttypes>

and accidentally changed a C file. The patch also removed #define __STDC_FORMAT_MACROS . crc32c_ppc.c does not use format macros, so I'm not putting that #define back.

As a consequence, MyRocks/MariaDB build fails on PPC now: https://jira.mariadb.org/browse/MDEV-19830

[miasantreble]

LGTM, thanks for the fix

[facebook-github-bot]

@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[riversand963]

Do we need the __STDC_FORMAT_MACROS?

[siying]

@riversand963 if it compiles, then we don't need it.

[miasantreble]

@spetrunia if this file doesn't use format macros then maybe we can simply remove the include?

[riversand963]

@riversand963 if it compiles, then we don't need it.

According to people's experience with different compilers, e.g. https://stackoverflow.com/questions/14535556/why-doesnt-priu64-work-in-this-code, if #include <inttypes.h> is present, we should always use __STDC_FORMAT_MACROS.

[siying]

@riversand963 I wonder whether __STDC_FORMAT_MACROS is a C++ thing or also is also applied to C. If it is a C++ thing, then we should not have it at all. The macro has name STD on it, so I suspect that it is a C++ thing.

[riversand963]

@siying , SG. Should not be necessary for C.

[spetrunia]

@spetrunia if this file doesn't use format macros then maybe we can simply remove the include?

Actually, seems like yes. I don't have access to a power8 machine, but examining the file manually, it looks like the #include <stdint.h> could be removed.

EDIT: Wait, crc32c_ppc.c uses uint32_t, which is according to https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html or https://en.cppreference.com/w/c/types/integer is defined in stdint.h . inttypes.h can also be used as it includes that file: http://pubs.opengroup.org/onlinepubs/009696799/basedefs/inttypes.h.html

Should this be done in the scope of this PR or the current variant is fine too?

[miasantreble]

Should this be done in the scope of this PR or the current variant is fine too?

I'd prefer removing unnecessary include directives

[spetrunia]

@miasantreble, changed the patch to include stdint.h. Please review

[facebook-github-bot]

@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@miasantreble merged this pull request in e731f44.

[adamretter]

@siying @gfosco this one is needed in 6.3 branch to be able to do releases for PPC, could you also check that it goes into newer branches too? For now I have manually picked it onto v6.3.6 to be able to build the release.