[webkit-reviews] review granted: [Bug 194213] [GLib] Stop URI-escaping file system representations : [Attachment 361041] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 194213: [GLib] Stop URI-escaping file system representations
https://bugs.webkit.org/show_bug.cgi?id=194213

Attachment 361041: Patch

https://bugs.webkit.org/attachment.cgi?id=361041&action=review




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 361041
  --> https://bugs.webkit.org/attachment.cgi?id=361041
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361041&action=review

> Source/WTF/wtf/glib/FileSystemGlib.cpp:66
> +    GUniquePtr<gchar> utf8(g_convert(representation, representationLength,
> +	   "UTF-8", filenameCharsets[0], nullptr, &utf8Length, nullptr));
> +    if (!utf8)
> +	   return { };

An error here would be difficult to catch, I think, I would get the error and
show a message with WTFLogAlways.

> Source/WTF/wtf/glib/FileSystemGlib.cpp:88
> +    GUniquePtr<gchar> representation(g_convert(utf8.data(), utf8.length(),
> +	   filenameCharsets[0], "UTF-8", nullptr, &representationLength,
nullptr));
> +    if (!representation)
> +	   return { };

Ditto.

> Source/WTF/wtf/glib/FileSystemGlib.cpp:96
> +    auto* data = representation.data();
> +    return !!data && data[0] != '\0';

I think CString should have an isEmpty() method, but anyway, why don't we just
ensure we return a null CString in case of invalid representation? Then we can
simply use isNull() instead of this