maemo.org - Talk

maemo.org - Talk (https://talk.maemo.org/index.php)
-   Maemo 5 / Fremantle (https://talk.maemo.org/forumdisplay.php?f=40)
-   -   [ANNOUNCE]Alarm UI replacement (https://talk.maemo.org/showthread.php?t=87823)

freemangordon 2012-11-16 10:30

Re: [ANNOUNCE]Alarm UI replacement
 
Another version (0.3.1) is out, another memory leak fixed.

reinob 2012-11-16 11:33

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by freemangordon (Post 1294896)
Another version (0.3.1) is out, another memory leak fixed.

Well spotted leak! :)

freemangordon 2012-11-16 11:58

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by reinob (Post 1294907)
Well spotted leak! :)

You looked at the commit?

reinob 2012-11-16 12:04

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by freemangordon (Post 1294912)
You looked at the commit?

Obviously! do you think I trust you or what?! :)))

freemangordon 2012-11-16 12:11

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by reinob (Post 1294914)
Obviously! do you think I trust you or what?! :)))

:D :D :D

The bad news is that it seems this is not the last one :(

I would appreciate if you (or someone else) help me find the remaining memory leaks.

The other thing that will be very useful, is a list of packages (if any) for which the guy that wrote the stock alarmUI is developer/maintainer. Those should be checked very carefully (if possible) ;)

reinob 2012-11-16 12:47

Re: [ANNOUNCE]Alarm UI replacement
 
@freemangordon,

in alarm_remove() you have:

Code:

    alarm_event_delete(a->alarm_event);
    g_slist_free(a->buttons);
    free(a);
    alarms = g_slist_remove(alarms,a);
    alarm_events_cnt--;

but you don't free the notification:
Code:

    alarm_event_delete(a->alarm_event);
    g_slist_free(a->buttons);
    if(a->notification) g_object_unref(G_OBJECT(a->notification))
    free(a);
    alarms = g_slist_remove(alarms,a);
    alarm_events_cnt--;

also, is it OK to free(a) before g_slist_remove(), I assume g_slist_remove doesn't use a (only the address, which is still there).

reinob 2012-11-16 12:54

Re: [ANNOUNCE]Alarm UI replacement
 
also, in alarm_notify()

sound_file is a strdup'd and then sent to alarm_notify_notification_calendar(), which calls notify_notification_set_hint_string().

Immediately after that, sound_file is g_free()'d. Hopefully notify_notification_set_hint_string() makes a copy of it, as otherwise the pointer to the string will be invalid (hey, could be the reason for some calendar events not ringing :)

reinob 2012-11-16 13:04

Re: [ANNOUNCE]Alarm UI replacement
 
in size_request_cb()

according to http://developer.gnome.org/pango/sta...rocessing.html, the object returned by pango_context_get_metrics has to be freed by the caller (pango_font_metrics_unref).

This is not the case. You can do it right after calculating "height".

freemangordon 2012-11-16 13:31

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by reinob (Post 1294924)
@freemangordon,

in alarm_remove() you have:

Code:

    alarm_event_delete(a->alarm_event);
    g_slist_free(a->buttons);
    free(a);
    alarms = g_slist_remove(alarms,a);
    alarm_events_cnt--;

but you don't free the notification:
Code:

    alarm_event_delete(a->alarm_event);
    g_slist_free(a->buttons);
    if(a->notification) g_object_unref(G_OBJECT(a->notification))
    free(a);
    alarms = g_slist_remove(alarms,a);
    alarm_events_cnt--;


by the time alarm_remove is called, a->notification should be unreffed and NULL already. Though I will put some check for that, thanks.

Quote:

also, is it OK to free(a) before g_slist_remove(), I assume g_slist_remove doesn't use a (only the address, which is still there).
it is ok, list contains only the address, it does not care what is behind this address

freemangordon 2012-11-16 13:35

Re: [ANNOUNCE]Alarm UI replacement
 
Quote:

Originally Posted by reinob (Post 1294926)
also, in alarm_notify()

sound_file is a strdup'd and then sent to alarm_notify_notification_calendar(), which calls notify_notification_set_hint_string().

Immediately after that, sound_file is g_free()'d. Hopefully notify_notification_set_hint_string() makes a copy of it, as otherwise the pointer to the string will be invalid (hey, could be the reason for some calendar events not ringing :)

This one is ok too, notify_notification_set_hint_string() uses g_value_set_string(), which makes a copy.


All times are GMT. The time now is 09:03.

vBulletin® Version 3.8.8