launchpad-integration

memory leaks in liblaunchpad-integration

Bug #483610 reported by c7d2f5c8667d26fffd5e7772d632c76d
8
This bug affects 2 people
Affects Status Importance Assigned to Milestone
launchpad-integration
Fix Released
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;
    }

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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 :)

Revision history for this message
c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount) wrote :

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

Revision history for this message
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.

Revision history for this message
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.

Robert Ancell (robert-ancell)
Changed in launchpad-integration:
importance: Undecided → Low
status: Confirmed → Triaged
Revision history for this message
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  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.