[Webkit-unassigned] [Bug 14811] [gtk] [request] add a webkit_gtk_page_go_to_history_item function

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14811


alp at atoker.com changed:

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




------- Comment #10 from alp at atoker.com  2008-01-07 15:28 PDT -------
(From update of attachment 18298)
This is looking good.

A few issues:

+    return g_strdup(title.utf8().data());

We can't return a newly allocated string as a const gchar*. Ideally we should
cache the string being returned (this is what we do elsewhere in the public
API). Depending on the situation there are different strategies but we have to
follow the GTK+ conventions here or programmers will cause leaks by not freeing
returned strings.

bdash notes that there's no need to check before deleting:

if (historyItemPriv->historyItem)
        delete historyItemPriv->historyItem

Now that we've decided to use NULL in API code, you shouldn't return 0 for a
const gchar* return, but NULL.

I didn't loook too carefully yet but kit() and core() are usually light
operations that don't make new allocations or add references, so not following
that expectation may lead to leaks.


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