Bug 174628 – [EME] Build failure with Clang-3.8 on InitDataRegistry.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Carlos Alberto Lopez Perez]

Building WPE with clang-3.8 gives this error:

Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o
FAILED: Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o 
/usr/lib/ccache/clang++   -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DBUILDING_WebCore -DDATA_DIR=\"share\" -DGETTEXT_PACKAGE=\"WPE\" -DHAVE_CONFIG_H=1 -DSTATICALLY_LINKED_WITH_PAL=1 -isystem ../DependenciesWPE/Root/include/gstreamer-1.0 -isystem ../DependenciesWPE/Root/include/glib-2.0 -isystem ../DependenciesWPE/Root/lib/glib-2.0/include  [.....]  Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o -MF Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o -c /home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp
ICECC[13551] 16:24:49: no server found behind given hostname 192.168.10.152:10245
ICECC[13551] 16:24:49: got exception 2 (192.168.10.152) 
/home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45:34: error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long') to 'unsigned int' in initializer list [-Wc++11-narrowing]
    String json { buffer.data(), buffer.size() };
                                 ^~~~~~~~~~~~~
/home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45:34: note: insert an explicit cast to silence this issue
    String json { buffer.data(), buffer.size() };
                                 ^~~~~~~~~~~~~
                                 static_cast<unsigned int>( )
1 error generated.

[Carlos Alberto Lopez Perez]

Created attachment 315793 [details]
Patch

[Michael Catanzaro]

Comment on attachment 315793 [details]
Patch

I'm not sure about this. Clearly a String cannot hold as much data as a SharedBuffer, but perhaps extractKeyIDsKeyids should return an error instead of silently truncating the data?

It's odd that the String class uses unsigned rather than size_t.

[Carlos Alberto Lopez Perez]

I'm also not sure.

[Zan Dobersek]

In practice it's unlikely that such a large JSON input would be passed in. If it was truncated, it's again unlikely that it would be correctly parsed  in the parseJSON() call.

But for the sake of tidiness the input SharedBuffer should have its size tested, returning immediately if it exceeds the maximum value that fits into the unsigned type. After that we can static_cast<> the size_t value to unsigned without an issue.

[Carlos Alberto Lopez Perez]

Created attachment 315917 [details]
Patch

[Zan Dobersek]

Comment on attachment 315917 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315917&action=review

> Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45
> +    if (buffer.size() > UINT_MAX)

I think std::numeric_limits<unsigned>::max() would fit better, but up to you.

[Carlos Alberto Lopez Perez]

Committed r219658: <http://trac.webkit.org/changeset/219658>