Bug 118228 – Build fix: use of long long in CoreIPC::ArgumentEncoder and CoreIPC::ArgumentDecoder

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Kwang Yul Seo]

While porting NetworkProcess to Gtk port, I encountered the following compile errors:

In file included from ../../Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.h:29:0,
                 from ../../Source/WebKit2/Platform/CoreIPC/ArgumentCoders.h:29,
                 from ../../Source/WebKit2/Shared/FileAPI/BlobRegistrationData.cpp:31:
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h: In instantiation of `static bool CoreIPC::ArgumentCoder<T>::decode(CoreIPC::ArgumentDecoder&, T&) [with T = long long int]':
../../Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.h:91:49:   required from `bool CoreIPC::ArgumentDecoder::decode(T&) [with T = long long int]'
../../Source/WebKit2/Shared/FileAPI/BlobRegistrationData.cpp:142:39:   required from here
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h:44:36: error: `decode' is not a member of `long long int'
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h: In instantiation of `static void CoreIPC::ArgumentCoder<T>::encode(CoreIPC::ArgumentEncoder&, const T&) [with T = long long int]':
../../Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.h:57:9:   required from `void CoreIPC::ArgumentEncoder::encode(const T&) [with T = long long int]'
../../Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.h:62:9:   required from `CoreIPC::ArgumentEncoder& CoreIPC::ArgumentEncoder::operator<<(const T&) [with T = long long int; CoreIPC::ArgumentEncoder = CoreIPC::ArgumentEncoder]'
../../Source/WebKit2/Shared/FileAPI/BlobRegistrationData.cpp:91:29:   required from here
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h:39:9: error: request for member `encode' in `t', which is of non-class type `const long long int'
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h: In static member function `static bool CoreIPC::ArgumentCoder<T>::decode(CoreIPC::ArgumentDecoder&, T&) [with T = long long int]':
../../Source/WebKit2/Platform/CoreIPC/ArgumentCoder.h:45:5: warning: control reaches end of non-void function [-Wreturn-type]
ICECC[1565] 15:39:58: Compiled on 192.168.100.227

int64_t fixes build.

[Kwang Yul Seo]

Created attachment 205785 [details]
Patch

[Anders Carlsson]

Comment on attachment 205785 [details]
Patch

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

> Source/WebCore/platform/network/BlobData.h:149
> -    long long offset;
> -    long long length;
> +    int64_t offset;
> +    int64_t length;
>      double expectedModificationTime;

I don't think we should change code in WebCore because of IPC requirements.

[Kwang Yul Seo]

(In reply to comment #2)
> I don't think we should change code in WebCore because of IPC requirements.

Okay. Then do you want to add long long type to ArgumentEncoder::encode and ArgumentDecoder::decode?  The problem with this approach is that int64_t and long long are the same type on some platforms. 

For example, the current Mac port does not have this problem because they are the same type. So adding ArgumentEncoder::encode(long long) might conflict with ArgumentEncoder::encode(int64_t) on these platforms. (can't be overloaded)

So unless if you have a strong requirement for using long long over int64_t, I suggest to use int64. I understand long long is preferred in some cases because long long is at least 64 bit wide while int64_t is exactly 64 bit wide. But I think 64 bit is wide enough for offset and length here.

Or do you have any other suggestion?

[Alexey Proskuryakov]

> Okay. Then do you want to add long long type to ArgumentEncoder::encode and ArgumentDecoder::decode?

Definitely not, IPC types should be all fixed size.

Personally, I'd be OK with the current patch, but the way to address the comment Anders made is to add explicit casts (with value checks) in BlobRegistrationData encode and decode functions.

[Anders Carlsson]

(In reply to comment #4)

> Personally, I'd be OK with the current patch, but the way to address the comment Anders made is to add explicit casts (with value checks) in BlobRegistrationData encode and decode functions.

Explicit casts sounds good.

[Kwang Yul Seo]

Created attachment 205820 [details]
Patch

[Kwang Yul Seo]

(In reply to comment #5)
> Explicit casts sounds good.

Done. Thanks for the suggestion!

[Kwang Yul Seo]

The patch is ready for review. Alexey and Anders, would you review the patch again?

[Kwang Yul Seo]

Committed r153352: <http://trac.webkit.org/changeset/153352>