[Webkit-unassigned] [Bug 73798] Upstream 6 files into WebCore/platform/blackberry

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Dec 10 01:30:13 PST 2011


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


Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #118519|review+                     |review-
               Flag|                            |




--- Comment #16 from Nikolas Zimmermann <zimmermann at kde.org>  2011-12-10 01:30:13 PST ---
(From update of attachment 118519)
View in context: https://bugs.webkit.org/attachment.cgi?id=118519&action=review

Oh, just saw Rob gave r+, I have a rather big concern, so I revert to r-:

> Source/WebCore/platform/blackberry/FileSystemBlackBerry.cpp:44
> +Vector<String> listDirectory(const String& path, const String& filter)
> +{
> +    Vector<String> entries;

Ouch, we now have three different versions of listDirectory, that all more ore less use the same opendir() based functionality.
Once in platform/efl/FileSystemEfl.cpp (our code seems to be a copy of it), and platform/posix/FileSystemPosix.cpp.

Could you break this patch up into another piece, and refactor the listDirectory method? We can 1:1 share with Efl I think, and maybe also posix/.
No need for us to have the maintenance burden alone :-)

And note: this code in general looks scary, and I'd rather avoid having possibly unsafe code in trunk.
If we want to add this as new code, it needs an intensive review...

The rest of this patch looks fine, so I suggest to leave out FileSysytemBlackberry from this patch, until it is properly refactored.

> Source/WebCore/platform/blackberry/SharedBufferBlackBerry.cpp:32
> +    return 0;

return nullptr;

> Source/WebCore/platform/blackberry/SharedTimerBlackBerry.cpp:61
> +        timer = new SharedTimerBlackBerry();

s/()//

> Source/WebCore/platform/blackberry/SharedTimerBlackBerry.cpp:73
> +    if (m_started) {

if (!m_started) early return.

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



More information about the webkit-unassigned mailing list