[webkit-reviews] review granted: [Bug 57307] [GTK] Fix leaked pointer in FontGtk.cpp : [Attachment 87452] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Ryuan Choi
<ryuan.choi at samsung.com>'s request for review:
Bug 57307: [GTK] Fix leaked pointer in FontGtk.cpp
https://bugs.webkit.org/show_bug.cgi?id=57307

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87452&action=review

This code probably needs quite a bit more cleanup in the future, but this is a
good incremental change. Please make the following changes before landing.

> Source/WebCore/platform/graphics/gtk/FontGtk.cpp:92
>      for (i = 0; i < aLength; i++) {

Moving the declaration of i here would be more idiomatic in C++.

> Source/WebCore/platform/graphics/gtk/FontGtk.cpp:135
> +	   g_free((gpointer)aText);

You do not need to cast here.

> Source/WebCore/platform/graphics/gtk/FontGtk.cpp:142
>      gint new_length = 0;

new_length --> newLength;

> Source/WebCore/platform/graphics/gtk/FontGtk.cpp:154
>      if (from > 0) {
>	   // discard the first 'from' characters
>	   // FIXME: we should do this before the conversion probably
> -	   gchar* str_left = g_utf8_offset_to_pointer(utf8, from);
> -	   gchar* tmp = g_strdup(str_left);
> -	   g_free(utf8);
> -	   utf8 = tmp;
> +	   gchar* strLeft = g_utf8_offset_to_pointer(utf8Text.get(), from);
> +	   utf8Text.set(g_strdup(strLeft));
>      }
>  
> -    gchar* pos = utf8;
> +    gchar* pos = utf8Text.get();

I think you can easily optimize this section by doing something like:

gchar* pos = utf8Text.get();
if (from > 0) {
    // FIXME: we should do this before the conversion probably
    pos = g_utf8_offset_to_pointer(pos, from);
}