Summary: | Move FileHandle to WebCore FileHandle.h | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Rollin <krollin> | ||||||||||
Component: | WebCore Misc. | Assignee: | Keith Rollin <krollin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, commit-queue, koivisto, ryanhaddad, sam | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Keith Rollin
2016-12-07 16:20:01 PST
Created attachment 296498 [details]
Patch
Comment on attachment 296498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296498&action=review > Source/WebCore/platform/FileHandle.cpp:40 > + , m_fileHandle(invalidPlatformFileHandle) This isn't necessary because of the initializer list in the header. > Source/WebCore/platform/FileHandle.cpp:47 > + , m_fileHandle(invalidPlatformFileHandle) I think this is confusing. Maybe we should = delete this constructor. > Source/WebCore/platform/FileHandle.cpp:69 > + m_fileHandle = invalidPlatformFileHandle; ditto (In reply to comment #2) > > Source/WebCore/platform/FileHandle.cpp:47 > > + , m_fileHandle(invalidPlatformFileHandle) > > I think this is confusing. Maybe we should = delete this constructor. > > > Source/WebCore/platform/FileHandle.cpp:69 > > + m_fileHandle = invalidPlatformFileHandle; > > ditto Heh. I concur. I originally had them "= delete", but tried adding them into this version in the interests of completeness. Created attachment 296547 [details]
Patch
Created attachment 296555 [details]
Patch
Created attachment 296562 [details]
Patch
Comment on attachment 296562 [details] Patch Clearing flags on attachment: 296562 Committed r209583: <http://trac.webkit.org/changeset/209583> All reviewed patches have been landed. Closing bug. This change appears to have broken the Windows build: https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/82532 C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\FileHandle.cpp(107): error C3861: 'vasprintf': identifier not found [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] OK, I'll fix it if you haven't already rolled it out. Fixed in Bug 165642. |