Summary: | Build fix: use of long long in CoreIPC::ArgumentEncoder and CoreIPC::ArgumentDecoder | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||||
Component: | WebKit2 | Assignee: | Kwang Yul Seo <skyul> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, ap, cdumez, sam | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 108832 | ||||||||
Attachments: |
|
Description
Kwang Yul Seo
2013-06-30 23:43:32 PDT
Created attachment 205785 [details]
Patch
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. (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? > 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.
(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. Created attachment 205820 [details]
Patch
(In reply to comment #5) > Explicit casts sounds good. Done. Thanks for the suggestion! The patch is ready for review. Alexey and Anders, would you review the patch again? Committed r153352: <http://trac.webkit.org/changeset/153352> |