[Webkit-unassigned] [Bug 27424] adding ConvertToGCharPrivate.h

[bugzilla-daemon at webkit.org]

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


Mark Rowe (bdash) <mrowe at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33088|review?                     |review-
               Flag|                            |




--- Comment #6 from Mark Rowe (bdash) <mrowe at apple.com>  2009-07-23 00:19:24 PDT ---
(From update of attachment 33088)
> Index: WebKit/gtk/gdom/ConvertToGCharPrivate.h

Based on your previous comments and the fact that this deals with WebCore
types, this sounds like a file that's not intended to be used outside of
building WebKit.  Is there a reason that it needs the "Private" suffix?

> +/*
> + * added under bug #27424 - 19jul2009
> + */
> +
> +/* convenience c++ functions for converting various different webkit
> + * string types into a utf8 glib string.
> + */

These comments don't add anything.

ConvertToGchar doesn't fit our naming conventions, which states that functions
should not start with an initial capital letter.  The "Gchar" strikes me as a
little odd too, but I guess that's what falls out of the lowercase "gchar"
being camel-cased.  Returning a raw pointer that expect the caller to free
seems to be asking for memory leaks.  Is there some way we can make this
interface safer?

> +inline gchar* ConvertToGchar(WebCore::String const& s)
> +{
> +    return g_strdup(s.utf8().data());
> +}

There should be a blank line between each function.

> +inline gchar* ConvertToGchar(WebCore::KURL const& s)
> +{
> +    return g_strdup(s.string().utf8().data());
> +}

This could be expressed more concisely as: return ConvertToGchar(s.string());

> +inline gchar* ConvertToGchar(const JSC::UString& s)
> +{
> +    return g_strdup(s.UTF8String().c_str());
> +}

> +inline gchar* ConvertToGchar(WebCore::AtomicString const&s)
> +{
> +    return g_strdup(s.string().utf8().data());
> +}

Missing a space after the &.  This could be expressed more concisely as: return
ConvertToGchar(s.string());

> +2008-11-30  lkcl  <lkcl at lkcl.net>

Please update this with the current date and your name.

> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        WARNING: NO TEST CASES ADDED OR CHANGED

This line is an intended only as an informative note for the patch author.

> +        (GStringConvert): bug #27424 - added ConvertToGCharPrivate.h to help GObject bindings convert various types to gobject gchar* 

Please take a look at the format of our other ChangeLog entries for how your
entry should be structured.  The prepare-ChangeLog script can be used to assist
in the creation of a ChangeLog entry for your change.

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