[webkit-reviews] review denied: [Bug 124987] Nix Upstream: Adding missing nix new files to WebCore : [Attachment 218020] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Thiago de Barros Lacerda
<thiago.lacerda at openbossa.org>'s request for review:
Bug 124987: Nix Upstream: Adding missing nix new files to WebCore
https://bugs.webkit.org/show_bug.cgi?id=124987

Attachment 218020: Patch
https://bugs.webkit.org/attachment.cgi?id=218020&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=218020&action=review


> Source/WebCore/platform/nix/FileSystemNix.cpp:49
> +/* On linux file names are just raw bytes, so also strings that cannot be
encoded in any way
> + * are valid file names. This mean that we cannot just store a file name
as-is in a String
> + * but we have to escape it.
> + * On Windows the GLib file name encoding is always UTF-8 so we can optimize
this case. */

This file is in WebCore/platform. It should use regular comment style.

> Source/WebCore/platform/nix/FileSystemNix.cpp:208
> +    static CString cachedPath;

This needs DEFINE_STATIC_LOCAL.

> Source/WebCore/platform/nix/FileSystemNix.cpp:230
> +    /* No null checking needed */

Comment style.

This comment does not add much. It does not explain the "why" of the code.

> Source/WebCore/platform/nix/MIMETypeRegistryNix.cpp:87
> +    String s = ext.lower();
> +    const ExtensionMap *e = extensionMap;
> +    while (e->extension) {
> +	   if (s == e->extension)
> +	       return e->mimeType;
> +	   ++e;
> +    }

Name the variables, "s" and "e" is no good.

> Source/WebCore/platform/nix/SharedTimerNix.cpp:45
> +void setSharedTimerFiredFunction(void (*f)())
> +{
> +    sharedTimerFiredFunction = f;
> +}

Name "f"?