Summary: | [Curl] Cookie Database files are wrongfully getting deleted when the database is opened | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Christopher Reid <chris.reid> | ||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, annulen, Basuke.Suzuki, bfulgham, commit-queue, don.olmstead, ews-watchlist, galpeter, pvollan, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Christopher Reid
2018-02-22 12:12:25 PST
Created attachment 334465 [details]
Patch
Comment on attachment 334465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334465&action=review LGTM. I'm not sure how liberally we should be using auto or not. > Source/WebCore/platform/network/curl/CookieJarDB.cpp:166 > + FileSystem::PlatformFileHandle handle = FileSystem::openFile(getCorruptionMarkerPath(), FileSystem::FileOpenMode::Write); Maybe auto here? (In reply to Don Olmstead from comment #2) > Comment on attachment 334465 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334465&action=review > > LGTM. I'm not sure how liberally we should be using auto or not. > > > Source/WebCore/platform/network/curl/CookieJarDB.cpp:166 > > + FileSystem::PlatformFileHandle handle = FileSystem::openFile(getCorruptionMarkerPath(), FileSystem::FileOpenMode::Write); > > Maybe auto here? It looks like auto is used quite a bit with openFile in the FileSystem implementation files so it makes sense to use it here too. I'll update the patch. Created attachment 334547 [details]
Patch
Comment on attachment 334547 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334547&action=review R=me. > Source/WebCore/platform/network/curl/CookieJarDB.cpp:167 > + FileSystem::closeFile(handle); Should we check whether 'handle' is valid before closing? Thanks for the review! (In reply to Per Arne Vollan from comment #5) > Comment on attachment 334547 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334547&action=review > > R=me. > > > Source/WebCore/platform/network/curl/CookieJarDB.cpp:167 > > + FileSystem::closeFile(handle); > > Should we check whether 'handle' is valid before closing? Ah, right now all the closeFile implementations have a check but it seems safer to have a check here too in case more functionality is added to that file. Created attachment 334550 [details]
Patch for landing
Comment on attachment 334550 [details] Patch for landing Clearing flags on attachment: 334550 Committed r228971: <https://trac.webkit.org/changeset/228971> All reviewed patches have been landed. Closing bug. |