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

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Amruth Raj <amruthraj at motorola.com>  2011-01-11 07:21:54 PST ---
(In reply to comment #3)
> (From update of attachment 78229 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=78229&action=review
> 
> Very nice start!
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:81
> > +    : m_fileName()
> 
> No need to use an empty initializer here.
Done.
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:121
> > +    char fileName[] = "/tmp/WK2SharedMemoryXXXXXX";
> > +    if (mkstemp(fileName) == -1) // create a unique name
> > +        return 0;
> 
> Please use GLib functions to do this.
Done.
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:126
> > +    RefPtr<SharedMemory> sharedMemory(adoptRef(new SharedMemory));
> > +    sharedMemory->m_size = size;
> > +    sharedMemory->m_data = createShareableMemory(fileName, size);
> > +    sharedMemory->m_fileName = String(fileName);
> 
> I would much prefer these to be arguments to the constructor, if possible. It's an error here to use cast a char* to a String. You should use filenameToString (from FileSystemGtk) or use a CString a m_fileName, if possible.
The port independent SharedMemory class doesn't define any constructor. Hence, I took this approach. Other ports implement the same way. 
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:149
> > +    int mode = memoryProtection(protection);
> 
> You only use this once, so no need to cache it in "mode"
Done.
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:150
> > +    char* fileName = const_cast<char*>(handle.m_fileName.utf8().data());
> 
> You cannot assume UTF-8 here. See above.
Done. I used fileSystemRepresentation over here.
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:155
> > +    RefPtr<SharedMemory> sharedMemory(adoptRef(new SharedMemory));
> > +    sharedMemory->m_size = size;
> > +    sharedMemory->m_data = obtainSharedMemory(fileName, size, mode);
> 
> Again, I think it's better to pass these as arguments to the constructor.
Same as above.
> 
> > WebKit2/Platform/gtk/SharedMemoryGtk.cpp:158
> > +    handle.m_fileName = String();
> 
> Why is this necessary? It should probably have a comment explaining it.
Removed this.

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