[Webkit-unassigned] [Bug 45416] Allow Chromium port to link with system-provided SQLite

[bugzilla-daemon at webkit.org]

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


Dumitru Daniliuc <dumi at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #68080|review?                     |review-
               Flag|                            |




--- Comment #9 from Dumitru Daniliuc <dumi at chromium.org>  2010-09-21 18:37:42 PST ---
(From update of attachment 68080)
View in context: https://bugs.webkit.org/attachment.cgi?id=68080&action=review

looks pretty good. a few more comments. and please check that all layout tests in LayoutTests/storage pass in chromium on linux and mac.

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:749
> +static int syncWrapper(int fd, bool fullSync)

change the return type of this function to 'bool', and rename 'rc' to 'synced', or something like that.

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:757
> +#if PLATFORM(MAC)
> +    if (fullSync)
> +        rc = fcntl(fd, F_FULLSYNC, 0);
> +#endif
> +    if (rc)
> +        rc = fsync(fd);

i don't think this code mirrors what's in full_fsync(). i think it should be more like:

   int rc = 1;
#if PLATFORM(MAC)
    if (fullSync)
        rc = fcntl(fd, F_FULLSYNC, 0);
    if (rc)
        rc = fsync(fd);
#else
    rc = fdatasync(fd);
#endif
    return rc;

(with the appropriate changes to make it return a boolean)

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:769
> +    if (syncWrapper(pFile->h, (flags & 0x0F) == SQLITE_SYNC_FULL)) {

let's add a isFullSync = flags & 0x0F == SQLITE_SYNC_FULL variable, since we need it a few lines below too.

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:1080
> +// We disallow loading DSOs inside the renderer process, so the following
> +// procedures are no-op.

1 line.

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:1085
> +static void chromiumDlError(sqlite3_vfs*, int, char*)

empty lines between these functions.

> WebCore/platform/sql/chromium/SQLiteFileSystemChromiumPosix.cpp:1153
> +static const int ChromiumMaxPathname = 512;

chromiumMaxPathname

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.