memory leaks in liblaunchpad-integration

Bug #483610 reported by c7d2f5c8667d26fffd5e7772d632c76d on 2009-11-16
8
This bug affects 2 people
Affects Status Importance Assigned to Milestone
launchpad-integration
Low
Unassigned

Bug Description

The code below leaks the return value (if non-NULL) of g_find_program_in_path(). It should be freed with g_free() instead.

void
launchpad_integration_add_ui (GtkUIManager *ui, const char *path)
{
    if (!g_find_program_in_path ("launchpad-integration")) {
        return;
    }
[...]

void
launchpad_integration_add_ui_with_separators (GtkUIManager *ui, const char *path,
                                              gboolean separator_before,
                                              gboolean separator_after)
{
    if (!g_find_program_in_path ("launchpad-integration")) {
        return;
    }

[...]
void
launchpad_integration_add_items (GtkWidget *helpmenu, int position,
                                 gboolean separator_before,
                                 gboolean separator_after)
{
    if (!g_find_program_in_path ("launchpad-integration")) {
        return;
    }

A. Bram Neijt (bneijt) wrote :

This may be one of the reasons for https://bugs.launchpad.net/gedit/+bug/305927

I think gedit calls this function pretty often, so that makes this a big problem. My gedit was using 700 MB of memory to look at about 10 files for about two days.

Hope this gets fixed soon.

A. Bram Neijt (bneijt) wrote :

Obvious if you look at the code, so confirmed. For other people with this problem, removing the launchpad-integration program will fix this as g_find_program_in_path will always return 0.

Changed in launchpad-integration:
status: New → Confirmed
A. Bram Neijt (bneijt) wrote :

Attached is an UNTESTED patch for this problem.

Reading http://library.gnome.org/devel/glib/unstable/glib-Miscellaneous-Utility-Functions.html
it turned out a bit more complex then I thought. If an absolute path is used, the free is not needed. A bugfix would also have been to use an absolute path for testing :)

You've misinterpreted the API docs. g_find_program_in_path() *always* returns a newly allocated string.

A. Bram Neijt (bneijt) wrote :

Sorry, re-reading the docs, that seems like the right thing. It returns a copy, so g_free-ing it should not be a problem. This means the if not g_path_is_absolute(name) can be left out. (Actually, it would cause a new memory leak if the function arguments where different :) )

Hope the bug gets fixed anyway.

A. Bram Neijt (bneijt) wrote :

Here is a new revision of the earlier patch, without the g_absolute_path check in it and using gboolean instead of int.

Changed in launchpad-integration:
importance: Undecided → Low
status: Confirmed → Triaged
Michael Terry (mterry) wrote :

This got merged and released a while ago.

Changed in launchpad-integration:
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers