Bug 156596 – Webkitgtk+ 2.12.1: Build Failure on slackware box: PRId64 not defined

• Component: WebKit Misc.
• Status: NEW

[David Ronis]

I'm trying to upgrade webkitgtk+ 2.12.1 on a Slackware 64 bit box.  The build dies (the log follows) having not found a definition for PRId64.   Grep shows several files in my include path
that contain the definition, and if I manually set it, the problem goes away.   Not surprisingly, the problem doesn't arise on my 32 bit machines.

Here's the log.

[ 54%] Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/platform/network/ParsedContentRange.cpp.o
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp: In member function ‘WTF::String WebCore::ParsedContentRange::headerValue() const’:
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:133:41: error: expected ‘)’ before ‘PRId64’
         return String::format("bytes %" PRId64 "-%" PRId64 "/*", m_firstBytePosition, m_lastBytePosition);
                                         ^
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:133:105: warning: spurious trailing ‘%’ in format [-Wformat=]
         return String::format("bytes %" PRId64 "-%" PRId64 "/*", m_firstBytePosition, m_lastBytePosition);
                                                                                                         ^
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:133:105: warning: too many arguments for format [-Wformat-extra-args]
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:134:37: error: expected ‘)’ before ‘PRId64’
     return String::format("bytes %" PRId64 "-%" PRId64 "/%" PRId64, m_firstBytePosition, m_lastBytePosition, m_instanceLength);
                                     ^
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:134:126: warning: spurious trailing ‘%’ in format [-Wformat=]
     return String::format("bytes %" PRId64 "-%" PRId64 "/%" PRId64, m_firstBytePosition, m_lastBytePosition, m_instanceLength);
                                                                                                                              ^
/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/Source/WebCore/platform/network/ParsedContentRange.cpp:134:126: warning: too many arguments for format [-Wformat-extra-args]
make[3]: *** [Source/WebCore/CMakeFiles/WebCore.dir/platform/network/ParsedContentRange.cpp.o] Error 1
make[3]: Leaving directory `/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/build'
make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/all] Error 2
make[2]: Leaving directory `/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/build'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/home/ronis/Project/notar/GNOME/garnome/platform/webkitgtk+/work/main.d/webkitgtk-2.12.1/build'
make: *** [build-work/main.d/webkitgtk-2.12.1/Makefile] Error 2

[Tomas Popela]

Created attachment 301029 [details]
Patch

[Konstantin Tokarev]

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

[Tomas Popela]

(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.

[Michael Catanzaro]

Why are we using inttypes.h at all? Can't we use cstdint instead?

[Michael Catanzaro]

Comment on attachment 301029 [details]
Patch

(Why are we using inttypes.h at all? Can't we use cstdint instead?)

[Tomas Popela]

(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.

[Michael Catanzaro]

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.