[webkit-reviews] review denied: [Bug 133069] [Curl] Invalid content in cache file, causes broken rendering. : [Attachment 231688] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 19 09:11:17 PDT 2014


Brent Fulgham <bfulgham at webkit.org> has denied peavo at outlook.com's request for
review:
Bug 133069: [Curl] Invalid content in cache file, causes broken rendering.
https://bugs.webkit.org/show_bug.cgi?id=133069

Attachment 231688: Patch
https://bugs.webkit.org/attachment.cgi?id=231688&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=231688&action=review


r-: Please adjust a few functions for early return! :-)

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:365
> +    if (m_contentFile == invalidPlatformFileHandle) {

We prefer this as an early return:  "if (m_contentFile !=
invalidPlatformFileHandle) \r return true;"

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:366
> +	   m_contentFile = openFile(m_contentFilename, OpenForWrite);

I was very surprised to see that we don't have an "OpenForWrite" style that
does not concatenate the file!

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:367
> +	   if (!isHandleValid(m_contentFile)) {

Again, we prefer to exit early if possible in tests like this.

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:377
> +    if (m_contentFile != invalidPlatformFileHandle) {

Early return please.


More information about the webkit-reviews mailing list