[Webkit-unassigned] [Bug 185873] [Curl] Not all Cookie database files are deleted on corruption

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=185873

--- Comment #7 from Christopher Reid <chris.reid at sony.com> ---
(In reply to Fujii Hironori from comment #4)
> Comment on attachment 341007 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341007&action=review
> 
> > Source/WebCore/platform/network/curl/CookieJarDB.cpp:222
> > +        return true;
> 
> Should this be a following code?
> if (resultCode != SQLITE_OK)
>     return false;
> 

Yeah, step also shouldn't even be returning SQLITE_OK.

> > Source/WebCore/platform/network/curl/CookieJarDB.cpp:249
> > +    FileSystem::deleteFile(m_databasePath + "-shm");
> 
> Why do you need to do 'deleteFile(m_databasePath + "-shm")' twice?
> 

Right, that shouldn't have been added.

> > Source/WebCore/platform/network/curl/CookieJarDB.cpp:510
> > +
> 
> You use checkSQLiteReturnCode in ASSERT macro even though it has a side
> effect.
> The side effect is effective only in Debug build.
> Is this intentional?
> 
> checkSQLiteReturnCode should be defined as:
> 
> void CookieJarDB::checkSQLiteReturnCode(int actual)
> {
>     if (!m_detectedDatabaseCorruption) {
>         switch (actual) {
>         case SQLITE_CORRUPT:
>         case SQLITE_SCHEMA:
>         case SQLITE_FORMAT:
>         case SQLITE_NOTADB:
>             flagDatabaseCorruption();
>             m_detectedDatabaseCorruption = true;
>         }
>     }
> }
> 
> This code should be replaced with:
> 
> checkSQLiteReturnCode(ret);
> if (ret != SQLITE_OK && ret != SQLITE_DONE && ret != SQLITE_ROW &&
> !ignoreError)
>     LOG_ERROR("Failed to execute %s error: %s", sql,
> m_database.lastErrorMsg());
> 
> Other ASSERT(checkSQLiteReturnCode(ret, SQLITE_DONE)) should be replaced
> with a following code:
> 
> checkSQLiteReturnCode(ret);
> ASSERT(ret == SQLITE_DONE);

That looks much better.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180816/5861255a/attachment-0001.html>