[Webkit-unassigned] [Bug 49791] [GTK] Implement SharedMemory for WebKit2

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-01-13 03:10:32 PST ---
(From update of attachment 78694)
View in context: https://bugs.webkit.org/attachment.cgi?id=78694&action=review

Unofficial review

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:48
> +    GIOChannel* ioChannel = g_io_channel_new_file(fileName, "w+", NULL);
> +    if (!ioChannel) 
> +        return 0;

Instead of using GIOChannel, it might be easier to use gio api and use g_seekable_truncate(). It might be something like:

GRefPtr<GFile> tmpFile = g_file_new_for_path(fileName);
GRefPtr<GFileIOStream> fileStream = g_file_open_readwrite(tmpFile.get(), 0, 0); // You might want to use a GError to handle errors here
g_seekable_truncate (G_SEEKABLE (fileStream.get()), size, 0, 0); // You might want to use a GError to handle errors here
g_io_stream_close(G_IO_STREAM(fileStream.get()), 0, 0); // You might want to use a GError to handle errors here

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:65
> +    GMappedFile* mappedFile = g_mapped_file_new(fileName, TRUE, NULL);
> +    if (!mappedFile)
> +        return 0;

When g_mapped_file_new() fails it returns NULL, so you can simply return g_mapped_file_new(fileName, TRUE, 0);

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:74
> +    GMappedFile* mappedFile = g_mapped_file_new(fileName, writable, NULL);
> +    if (!mappedFile)
> +        return 0;

The same here, I think we don't even need this method, we can just use g_mapped_file_new() directly

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:117


This is not portable, use g_get_tmp_dir()

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:118
> +    if (g_mkstemp(fileName) == -1) // create a unique name

Note that g_mkstemp not only creates a unique name it also opens the file and returns the fd. The file should closed with close (fd).

> WebKit2/Platform/gtk/SharedMemoryGtk.cpp:162
> +    g_mapped_file_unref(m_mappedFile);

We should probably add support for GMappedFile file to GRefPtr

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