[webkit-reviews] review granted: [Bug 182066] Add logging to facilitate binding of WebContent and Network processes to UI process : [Attachment 332210] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 24 19:05:28 PST 2018


Brent Fulgham <bfulgham at webkit.org> has granted Keith Rollin
<krollin at apple.com>'s request for review:
Bug 182066: Add logging to facilitate binding of WebContent and Network
processes to UI process
https://bugs.webkit.org/show_bug.cgi?id=182066

Attachment 332210: Patch

https://bugs.webkit.org/attachment.cgi?id=332210&action=review




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 332210
  --> https://bugs.webkit.org/attachment.cgi?id=332210
Patch

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

r=me if you remove the cast and just use %d.

>> Source/WebKit/NetworkProcess/NetworkProcess.cpp:263
>> +	RELEASE_LOG(Process, "%p - NetworkProcess::initializeNetworkProcess:
Presenting process = %lu", this, static_cast<unsigned
long>(WebCore::presentingApplicationPID()));
> 
> Seems like a good idea, but why are you casting from int to unsigned long and
using %lu, instead of just using %d? I know it's weird that
WebCore::presentingApplicationPID uses int rather than pid_t or unsigned int...
but that really is its type.

That is weird! Especially considering that it is a pid_t in many places. Maybe
Andy can tell us why it's just an int in this method. But for this logging
purpose, int seems reasonable.


More information about the webkit-reviews mailing list