Comment 2 for bug 724554

This doesn't appear to be completely fixed. The icon name is passed to foo_set_icon, which is apparently supposed to free it. However, the responsibility to free the icon name is not clearly documented in the code, nor does the function actually do it in all cases.

Since there are conditional compilation sections in foo_set_icon, it is somewhat difficult to follow. It appears that the only place the icon name gets freed is on line 2096, and that is inside an if-block inside the else condition of an ifdef block. Not only that, the function has several early returns and the call to g_free won't happen for those either.

Here is an example valgrind log of the current code:

==10301== 16 bytes in 1 blocks are definitely lost in loss record 2,425 of 9,326
==10301== at 0x4C290A4: realloc (vg_replace_malloc.c:525)
==10301== by 0x8F62B20: g_realloc (gmem.c:233)
==10301== by 0x8F7DCD6: g_string_maybe_expand (gstring.c:401)
==10301== by 0x8F7E3B6: g_string_insert_len (gstring.c:741)
==10301== by 0x8F7E5D4: g_string_assign (gstring.c:615)
==10301== by 0x428F39: get_best_icon_name_for_ap (applet-device-wifi.c:369)
==10301== by 0x42909B: wireless_get_icon (applet-device-wifi.c:1489)
==10301== by 0x41D252: applet_update_icon (applet.c:2605)
==10301== by 0x8F5BBCC: g_main_context_dispatch (gmain.c:2440)
==10301== by 0x8F5C3A7: g_main_context_iterate.clone.6 (gmain.c:3091)
==10301== by 0x8F5C9F1: g_main_loop_run (gmain.c:3299)
==10301== by 0x416D77: main (main.c:101)

Because the structure of foo_set_icon is not suited to guarantee that it frees the icon name, I suggest removing the call to g_free from foo_set_icon, and adding calls to g_free in applet_update_icon instead.

Patch attached. The patch is against r66, after the debian patch series has been applied. Probably it should be merged with the nm-applet-use-indicator patch, but I think a patch against a patch would have been very hard to read.