Summary: | Webkitgtk+ 2.12.1: Build Failure on slackware box: PRId64 not defined | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Ronis <David.Ronis> | ||||
Component: | WebKit Misc. | Assignee: | Tomas Popela <tpopela> | ||||
Status: | NEW --- | ||||||
Severity: | Major | CC: | alecflett, annulen, beidson, benjamin, cdumez, cgarcia, cmarcelo, commit-queue, dbates, jsbell, mcatanzaro, tpopela | ||||
Priority: | P2 | ||||||
Version: | Other | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
David Ronis
2016-04-14 13:55:09 PDT
Created attachment 301029 [details]
Patch
I wonder if we should define __STDC_FORMAT_MACROS in build system, so we don't have to track each <inttypes.h> occurence. Previously all places which needed these macros obtained it via wtf/Assertions.h, but now it's not not enough (In reply to comment #2) > I wonder if we should define __STDC_FORMAT_MACROS in build system, so we > don't have to track each <inttypes.h> occurence. Yes, that's also the way how to do it. I don't have a strong preference on the solution. > Previously all places which needed these macros obtained it via > wtf/Assertions.h, but now it's not not enough Yes it could be moved to Assertions.h, but I personally hate if I look in the includes (in StdLibExtras.h), where the symbol is used, but I don't find the include there, but I have to go through all the other includes. But that's just my POV. Why are we using inttypes.h at all? Can't we use cstdint instead? Comment on attachment 301029 [details]
Patch
(Why are we using inttypes.h at all? Can't we use cstdint instead?)
(In reply to comment #4) > Why are we using inttypes.h at all? Can't we use cstdint instead? Why can't use cstdint as the PRId64 and other print macros are defined in inttypes.h. Comment on attachment 301029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301029&action=review > Source/WTF/wtf/StdLibExtras.h:33 > +#define __STDC_FORMAT_MACROS > +#include <inttypes.h> I found http://en.cppreference.com/w/cpp/types/integer "The C99 standard suggests that C++ implementations should not define the above limit, constant, or format macros unless the macros __STDC_LIMIT_MACROS, __STDC_CONSTANT_MACROS or __STDC_FORMAT_MACROS (respectively) are defined before including the relevant C header (stdint.h or inttypes.h). This recommendation was not adopted by any C++ standard and was removed in C11. However, some implementations (such as glibc 2.17) try to apply this rule, and it may be necessary to define the __STDC macros; C++ compilers may try to work around this by automatically defining them in some circumstances." I would include cinttypes instead of inttypes.h, which takes care of defining this for you. But only include it here if it's really needed inside StdLibExtras.h itself. It doesn't look like it is. If you want to be comprehensive and avoid this problem in the future, a global replace of all use of inttypes.h with cinttypes would be advisable. Perhaps we should do that for all C standard library headers anyway, to replace them with their C++ counterparts. Right now there seems to be no rhyme or reason behind whether we use a C header or the corresponding C++ header. > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:59 > +#include <wtf/StdLibExtras.h> This is overkill to get some format specifiers; you should directly include cinttypes instead. |