From 37e796d3cb46f8292d85c9c68439a80d3e394960 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 20:00:14 -0600 Subject: extract method on common code --- src/datetime-service.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index ef43594..17608b9 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -331,7 +331,7 @@ update_datetime (gpointer user_data) /* Run a particular program based on an activation */ static void -activate_cb (DbusmenuMenuitem * menuitem, guint timestamp, const gchar *command) +execute_command (const gchar * command) { GError * error = NULL; @@ -342,6 +342,15 @@ activate_cb (DbusmenuMenuitem * menuitem, guint timestamp, const gchar *command) } } +/* Run a particular program based on an activation */ +static void +activate_cb (DbusmenuMenuitem * menuitem G_GNUC_UNUSED, + guint timestamp G_GNUC_UNUSED, + const gchar * command) +{ + execute_command (command); +} + static gboolean update_appointment_menu_items_idle (gpointer user_data) { @@ -420,13 +429,7 @@ day_selected_double_click_cb (DbusmenuMenuitem * menuitem, gchar *name, GVariant gchar *ad = isodate_from_time_t(evotime); gchar *cmd = g_strconcat("evolution calendar:///?startdate=", ad, NULL); - GError * error = NULL; - - g_debug("Issuing command '%s'", cmd); - if (!g_spawn_command_line_async(cmd, &error)) { - g_warning("Unable to start %s: %s", (char *)cmd, error->message); - g_error_free(error); - } + execute_command (cmd); return TRUE; } -- cgit v1.2.3 From 25e9f143eef355cbcb4b3cb7563e28414c4a93a4 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 20:03:28 -0600 Subject: fix memory leaks in day_selected_double_click_cb() --- src/datetime-service.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 17608b9..3416008 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -420,9 +420,12 @@ day_selected_cb (DbusmenuMenuitem * menuitem, gchar *name, GVariant *variant, gu } static gboolean -day_selected_double_click_cb (DbusmenuMenuitem * menuitem, gchar *name, GVariant *variant, guint timestamp) +day_selected_double_click_cb (DbusmenuMenuitem * menuitem G_GNUC_UNUSED, + gchar * name G_GNUC_UNUSED, + GVariant * variant, + guint timestamp G_GNUC_UNUSED) { - time_t evotime = (time_t)g_variant_get_uint32(variant); + const time_t evotime = (time_t)g_variant_get_uint32(variant); g_debug("Received day-selected-double-click with timestamp: %d -> %s",(int)evotime, ctime(&evotime)); @@ -430,6 +433,9 @@ day_selected_double_click_cb (DbusmenuMenuitem * menuitem, gchar *name, GVariant gchar *cmd = g_strconcat("evolution calendar:///?startdate=", ad, NULL); execute_command (cmd); + + g_free (cmd); + g_free (ad); return TRUE; } -- cgit v1.2.3 From 9e8aae213e7a3ced1b6ec8b6b48a9bd69532e709 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 20:19:21 -0600 Subject: fix memory leak in update_appointment_menu_items() --- src/datetime-service.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 3416008..438bf5d 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -727,7 +727,6 @@ update_appointment_menu_items (gpointer user_data) updating_appointments = TRUE; time_t curtime = 0, t1 = 0, t2 = 0; - gchar *ad; GList *l; GSList *g; GError *gerror = NULL; @@ -894,7 +893,7 @@ update_appointment_menu_items (gpointer user_data) struct comp_instance *ci = l->data; ECalComponent *ecalcomp = ci->comp; ECalComponentText valuetext; - gchar *summary, *cmd; + gchar *summary; char right[20]; //const gchar *uri; DbusmenuMenuitem * item; @@ -995,12 +994,12 @@ update_appointment_menu_items (gpointer user_data) // Now we pull out the URI for the calendar event and try to create a URI that'll work when we execute evolution // FIXME Because the URI stuff is really broken, we're going to open the calendar at todays date instead //e_cal_component_get_uid(ecalcomp, &uri); - ad = isodate_from_time_t(mktime(due)); - cmd = g_strconcat("evolution calendar:///?startdate=", ad, NULL); - g_signal_connect (G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, - G_CALLBACK (activate_cb), cmd); - + gchar * ad = isodate_from_time_t(mktime(due)); + gchar * cmd = g_strconcat("evolution calendar:///?startdate=", ad, NULL); g_debug("Command to Execute: %s", cmd); + g_signal_connect_data (G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, + G_CALLBACK(activate_cb), cmd, (GClosureNotify)g_free, 0); + g_free (ad); const gchar *color_spec = e_source_peek_color_spec(ci->source); g_debug("Colour to use: %s", color_spec); -- cgit v1.2.3 From a8a5a7913ccac164bb8abe4dad8afc9bb736d665 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 20:23:22 -0600 Subject: remove unnecessary strdup+free in update_appointment_menu_items() --- src/datetime-service.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 438bf5d..a1b9c69 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -892,8 +892,6 @@ update_appointment_menu_items (gpointer user_data) for (l = sorted_comp_instances; l; l = l->next) { struct comp_instance *ci = l->data; ECalComponent *ecalcomp = ci->comp; - ECalComponentText valuetext; - gchar *summary; char right[20]; //const gchar *uri; DbusmenuMenuitem * item; @@ -950,12 +948,11 @@ update_appointment_menu_items (gpointer user_data) // Label text + ECalComponentText valuetext; e_cal_component_get_summary (ecalcomp, &valuetext); - summary = g_strdup (valuetext.value); - - dbusmenu_menuitem_property_set (item, APPOINTMENT_MENUITEM_PROP_LABEL, summary); + const gchar * summary = valuetext.value; g_debug("Summary: %s", summary); - g_free (summary); + dbusmenu_menuitem_property_set (item, APPOINTMENT_MENUITEM_PROP_LABEL, summary); gboolean full_day = FALSE; if (vtype == E_CAL_COMPONENT_EVENT) { -- cgit v1.2.3 From cf8ba28a080998c29e415abeda4277452539cc49 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 20:30:07 -0600 Subject: fix potential minor memory leak in update_timezone_menu_items() --- src/datetime-service.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/datetime-service.c b/src/datetime-service.c index a1b9c69..4edd3e7 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -603,6 +603,7 @@ update_timezone_menu_items(gpointer user_data) { dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); if (len == 0) { + g_strfreev (locations); g_debug("No locations configured (Empty List)"); return FALSE; } -- cgit v1.2.3 From 4586df056f8a637c9a33d1d6a70ca0480b2c9e0c Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:01:49 -0600 Subject: tweak: use g_clear_object() in dispose() --- src/indicator-datetime.c | 52 +++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/src/indicator-datetime.c b/src/indicator-datetime.c index c847d47..08d7383 100644 --- a/src/indicator-datetime.c +++ b/src/indicator-datetime.c @@ -405,10 +405,7 @@ service_proxy_cb (GObject * object, GAsyncResult * res, gpointer user_data) GDBusProxy * proxy = g_dbus_proxy_new_for_bus_finish(res, &error); - if (priv->service_proxy_cancel != NULL) { - g_object_unref(priv->service_proxy_cancel); - priv->service_proxy_cancel = NULL; - } + g_clear_object (&priv->service_proxy_cancel); if (error != NULL) { g_warning("Could not grab DBus proxy for %s: %s", SERVICE_NAME, error->message); @@ -429,46 +426,25 @@ static void indicator_datetime_dispose (GObject *object) { IndicatorDatetime * self = INDICATOR_DATETIME(object); + IndicatorDatetimePrivate * priv = self->priv; - if (self->priv->label != NULL) { - g_object_unref(self->priv->label); - self->priv->label = NULL; - } - - if (self->priv->timer != 0) { - g_source_remove(self->priv->timer); - self->priv->timer = 0; - } - - if (self->priv->idle_measure != 0) { - g_source_remove(self->priv->idle_measure); - self->priv->idle_measure = 0; - } - - if (self->priv->menu != NULL) { - g_object_unref(G_OBJECT(self->priv->menu)); - self->priv->menu = NULL; - } - - if (self->priv->sm != NULL) { - g_object_unref(G_OBJECT(self->priv->sm)); - self->priv->sm = NULL; + if (priv->timer != 0) { + g_source_remove(priv->timer); + priv->timer = 0; } - if (self->priv->settings != NULL) { - g_object_unref(G_OBJECT(self->priv->settings)); - self->priv->settings = NULL; + if (priv->idle_measure != 0) { + g_source_remove(priv->idle_measure); + priv->idle_measure = 0; } - if (self->priv->service_proxy != NULL) { - g_object_unref(self->priv->service_proxy); - self->priv->service_proxy = NULL; - } + g_clear_object (&priv->label); + g_clear_object (&priv->menu); + g_clear_object (&priv->sm); + g_clear_object (&priv->settings); + g_clear_object (&priv->service_proxy); + g_clear_object (&priv->indicator_right_group); - if (self->priv->indicator_right_group != NULL) { - g_object_unref(G_OBJECT(self->priv->indicator_right_group)); - self->priv->indicator_right_group = NULL; - } G_OBJECT_CLASS (indicator_datetime_parent_class)->dispose (object); return; -- cgit v1.2.3 From 8be0daf8d37a2c53df8f097c389e35c7d4c5ea6b Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:02:57 -0600 Subject: in dispose(), add g_clear_object() for priv.ido_calendar and priv.service_proxy_cancel --- src/indicator-datetime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/indicator-datetime.c b/src/indicator-datetime.c index 08d7383..91a029c 100644 --- a/src/indicator-datetime.c +++ b/src/indicator-datetime.c @@ -444,7 +444,8 @@ indicator_datetime_dispose (GObject *object) g_clear_object (&priv->settings); g_clear_object (&priv->service_proxy); g_clear_object (&priv->indicator_right_group); - + g_clear_object (&priv->ido_calendar); + g_clear_object (&priv->service_proxy_cancel); G_OBJECT_CLASS (indicator_datetime_parent_class)->dispose (object); return; -- cgit v1.2.3 From 176b64b57d0f19af418d88199bf593f96a5a12a7 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:15:35 -0600 Subject: more use of g_clear_object() where appropriate --- src/datetime-interface.c | 13 +++---------- src/datetime-prefs.c | 49 +++++++++++++++++++++--------------------------- 2 files changed, 24 insertions(+), 38 deletions(-) diff --git a/src/datetime-interface.c b/src/datetime-interface.c index 5939061..e67be85 100644 --- a/src/datetime-interface.c +++ b/src/datetime-interface.c @@ -124,10 +124,7 @@ bus_get_cb (GObject * object, GAsyncResult * res, gpointer user_data) g_warn_if_fail(priv->bus == NULL); priv->bus = connection; - if (priv->bus_cancel != NULL) { - g_object_unref(priv->bus_cancel); - priv->bus_cancel = NULL; - } + g_clear_object (&priv->bus_cancel); /* Now register our object on our new connection */ priv->dbus_registration = g_dbus_connection_register_object(priv->bus, @@ -158,15 +155,11 @@ datetime_interface_dispose (GObject *object) priv->dbus_registration = 0; } - if (priv->bus != NULL) { - g_object_unref(priv->bus); - priv->bus = NULL; - } + g_clear_object (&priv->bus); if (priv->bus_cancel != NULL) { g_cancellable_cancel(priv->bus_cancel); - g_object_unref(priv->bus_cancel); - priv->bus_cancel = NULL; + g_clear_object (&priv->bus_cancel); } G_OBJECT_CLASS (datetime_interface_parent_class)->dispose (object); diff --git a/src/datetime-prefs.c b/src/datetime-prefs.c index 9fdfbed..50445ff 100644 --- a/src/datetime-prefs.c +++ b/src/datetime-prefs.c @@ -776,46 +776,39 @@ static void indicator_datetime_panel_dispose (GObject * object) { IndicatorDatetimePanel * self = (IndicatorDatetimePanel *) object; + IndicatorDatetimePanelPrivate * priv = self->priv; - if (self->priv->builder) { - g_object_unref (self->priv->builder); - self->priv->builder = NULL; - } - - if (self->priv->proxy) { - g_object_unref (self->priv->proxy); - self->priv->proxy = NULL; - } + g_clear_object (&priv->builder); + g_clear_object (&priv->proxy); - if (self->priv->loc_dlg) { - gtk_widget_destroy (self->priv->loc_dlg); - self->priv->loc_dlg = NULL; + if (priv->loc_dlg) { + gtk_widget_destroy (priv->loc_dlg); + priv->loc_dlg = NULL; } - if (self->priv->save_time_id) { - g_source_remove (self->priv->save_time_id); - self->priv->save_time_id = 0; + if (priv->save_time_id) { + g_source_remove (priv->save_time_id); + priv->save_time_id = 0; } - if (self->priv->completion) { - cc_timezone_completion_watch_entry (self->priv->completion, NULL); - g_object_unref (self->priv->completion); - self->priv->completion = NULL; + if (priv->completion) { + cc_timezone_completion_watch_entry (priv->completion, NULL); + g_clear_object (&priv->completion); } - if (self->priv->tz_entry) { - gtk_widget_destroy (self->priv->tz_entry); - self->priv->tz_entry = NULL; + if (priv->tz_entry) { + gtk_widget_destroy (priv->tz_entry); + priv->tz_entry = NULL; } - if (self->priv->time_spin) { - gtk_widget_destroy (self->priv->time_spin); - self->priv->time_spin = NULL; + if (priv->time_spin) { + gtk_widget_destroy (priv->time_spin); + priv->time_spin = NULL; } - if (self->priv->date_spin) { - gtk_widget_destroy (self->priv->date_spin); - self->priv->date_spin = NULL; + if (priv->date_spin) { + gtk_widget_destroy (priv->date_spin); + priv->date_spin = NULL; } } -- cgit v1.2.3 From e2cd48965d699a4b3b246db1ae1dc3cb073901d8 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:17:49 -0600 Subject: make the private fields 'conf' and 'gconf' static and init them to NULL --- src/datetime-service.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 4edd3e7..6cd2259 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -84,13 +84,13 @@ static DbusmenuMenuitem * geo_location = NULL; static DbusmenuMenuitem * current_location = NULL; //static DbusmenuMenuitem * ecal_location = NULL; static DbusmenuMenuitem * add_appointment = NULL; -static GList * appointments = NULL; -static GList * dconflocations = NULL; -static GList * comp_instances = NULL; +static GList * appointments = NULL; +static GList * dconflocations = NULL; +static GList * comp_instances = NULL; static gboolean updating_appointments = FALSE; -static time_t start_time_appointments = (time_t) 0; -GSettings *conf; -GConfClient* gconf; +static time_t start_time_appointments = (time_t) 0; +static GSettings * conf = NULL; +static GConfClient * gconf = NULL; /* Geoclue trackers */ -- cgit v1.2.3 From 817baff6346d7910bd2d006457b81717ef1e0137 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:32:02 -0600 Subject: make update_timezone_menu_items() a void function; its args and return value were unused --- src/datetime-service.c | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 6cd2259..87d537e 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -62,7 +62,7 @@ with this program. If not, see . static void geo_create_client (GeoclueMaster * master, GeoclueMasterClient * client, gchar * path, GError * error, gpointer user_data); static gboolean update_appointment_menu_items (gpointer user_data); -static gboolean update_timezone_menu_items(gpointer user_data); +static void update_location_menu_items (void); static void setup_timer (void); static void geo_client_invalid (GeoclueMasterClient * client, gpointer user_data); static void geo_address_change (GeoclueMasterClient * client, gchar * a, gchar * b, gchar * c, gchar * d, gpointer user_data); @@ -168,7 +168,7 @@ check_timezone_sync (void) { dbusmenu_menuitem_property_set_bool(locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - update_timezone_menu_items(NULL); // Update the timezone menu items + update_location_menu_items(); return; } @@ -214,7 +214,7 @@ check_timezone_sync (void) { } } g_debug("Finished checking timezone sync"); - update_timezone_menu_items(NULL); // Update the timezone menu items + update_location_menu_items(); return; } @@ -567,52 +567,46 @@ check_for_calendar (gpointer user_data) } -static gboolean -update_timezone_menu_items(gpointer user_data) { +static void +update_location_menu_items(void) { g_debug("Updating timezone menu items"); if (locations_separator == NULL || current_location == NULL) { - return FALSE; + return; } gchar ** locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S); if (locations == NULL) { g_debug("No locations configured (NULL)"); - return FALSE; + return; } - guint len = g_strv_length(locations); - DbusmenuMenuitem *item; - gint i, offset; + const guint len = g_strv_length(locations); + g_debug("Found %u locations from %s", len, SETTINGS_LOCATIONS_S); /* Remove all of the previous locations */ if (dconflocations != NULL) { while (dconflocations != NULL) { - DbusmenuMenuitem * litem = DBUSMENU_MENUITEM(dconflocations->data); + DbusmenuMenuitem * item = DBUSMENU_MENUITEM(dconflocations->data); // Remove all the existing menu items which are in dconflocations. - dconflocations = g_list_remove(dconflocations, litem); - dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(litem)); - g_object_unref(G_OBJECT(litem)); + dconflocations = g_list_remove(dconflocations, item); + dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item)); + g_object_unref(G_OBJECT(item)); } } - gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); - + const gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show); dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show); dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); - if (len == 0) { - g_strfreev (locations); - g_debug("No locations configured (Empty List)"); - return FALSE; - } - - offset = dbusmenu_menuitem_get_position (current_location, root)+1; + gint i; + gint offset = dbusmenu_menuitem_get_position (current_location, root)+1; for (i = 0; i < len; i++) { // Iterate over configured places and add any which aren't already listed if ((current_timezone == NULL || !g_str_has_prefix(locations[i], current_timezone)) && (geo_timezone == NULL || !g_str_has_prefix(locations[i], geo_timezone))) { + DbusmenuMenuitem *item; g_debug("Adding timezone in update_timezones %s", locations[i]); item = dbusmenu_menuitem_new(); dbusmenu_menuitem_property_set (item, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE); @@ -626,7 +620,6 @@ update_timezone_menu_items(gpointer user_data) { } } g_strfreev (locations); - return FALSE; } // Authentication function -- cgit v1.2.3 From 53b0569f05784e3d22af00be9b11a6c2f1a8f9aa Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 1 Mar 2012 21:56:25 -0600 Subject: rename dconflocations as location_menu_items --- src/datetime-service.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 87d537e..b1b5b7f 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -85,7 +85,7 @@ static DbusmenuMenuitem * current_location = NULL; //static DbusmenuMenuitem * ecal_location = NULL; static DbusmenuMenuitem * add_appointment = NULL; static GList * appointments = NULL; -static GList * dconflocations = NULL; +static GList * location_menu_items = NULL; static GList * comp_instances = NULL; static gboolean updating_appointments = FALSE; static time_t start_time_appointments = (time_t) 0; @@ -585,14 +585,11 @@ update_location_menu_items(void) { g_debug("Found %u locations from %s", len, SETTINGS_LOCATIONS_S); /* Remove all of the previous locations */ - if (dconflocations != NULL) { - while (dconflocations != NULL) { - DbusmenuMenuitem * item = DBUSMENU_MENUITEM(dconflocations->data); - // Remove all the existing menu items which are in dconflocations. - dconflocations = g_list_remove(dconflocations, item); - dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item)); - g_object_unref(G_OBJECT(item)); - } + while (location_menu_items != NULL) { + DbusmenuMenuitem * item = DBUSMENU_MENUITEM(location_menu_items->data); + location_menu_items = g_list_remove(location_menu_items, item); + dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item)); + g_object_unref(G_OBJECT(item)); } const gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); @@ -616,7 +613,7 @@ update_location_menu_items(void) { dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, show); dbusmenu_menuitem_child_add_position (root, item, offset++); g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL); - dconflocations = g_list_append(dconflocations, item); + location_menu_items = g_list_append(location_menu_items, item); } } g_strfreev (locations); -- cgit v1.2.3 From ece0242ddfa2e79eebdc70a1c494a82640ee7fdc Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 2 Mar 2012 14:12:20 -0600 Subject: remove duplicate timezone entries --- src/datetime-service.c | 283 ++++++++++++++++++++++++------------------------- 1 file changed, 140 insertions(+), 143 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index b1b5b7f..710b7d6 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -68,6 +68,8 @@ static void geo_client_invalid (GeoclueMasterClient * client, gpointer user_data static void geo_address_change (GeoclueMasterClient * client, gchar * a, gchar * b, gchar * c, gchar * d, gpointer user_data); static gboolean get_greeter_mode (void); +static void quick_set_tz (DbusmenuMenuitem * menuitem, guint timestamp, gpointer user_data); + static IndicatorService * service = NULL; static GMainLoop * mainloop = NULL; static DbusmenuServer * server = NULL; @@ -109,114 +111,162 @@ struct comp_instance { }; static void -set_timezone_label (DbusmenuMenuitem * mi, const gchar * location) +set_current_timezone_label (DbusmenuMenuitem * mi, const gchar * location) { - gchar * zone, * name; - split_settings_location (location, &zone, &name); + gchar * name = get_current_zone_name (location); dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_NAME, name); - dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, zone); + dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, location); - g_free (zone); g_free (name); } +/** + * A temp struct used by update_location_menu_items() for pruning duplicates. + */ +struct NameAndOffset +{ + gchar * name; + gint32 offset; +}; static void -set_current_timezone_label (DbusmenuMenuitem * mi, const gchar * location) +name_and_offset_free (struct NameAndOffset * nao) { - gchar * name = get_current_zone_name (location); - - dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_NAME, name); - dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, location); - - g_free (name); + g_free (nao->name); + g_free (nao); } +static struct NameAndOffset* +name_and_offset_new (const char * zone, const char * name) +{ + struct NameAndOffset * nao = g_new (struct NameAndOffset, 1); + nao->name = g_strdup (name); + GTimeZone * tz = g_time_zone_new (zone); + nao->offset = g_time_zone_get_offset (tz, 0); + g_time_zone_unref (tz); + return nao; +} +static struct NameAndOffset* +name_and_offset_new_from_item (DbusmenuMenuitem * mi) +{ + const char * zone = dbusmenu_menuitem_property_get (mi, TIMEZONE_MENUITEM_PROP_ZONE); + const char * name = dbusmenu_menuitem_property_get (mi, TIMEZONE_MENUITEM_PROP_NAME); + return name_and_offset_new (zone, name); +} +static int +name_and_offset_compare (const struct NameAndOffset * a, const struct NameAndOffset * b) +{ + if (a->offset != b->offset) + return a->offset - b->offset; + return g_strcmp0 (a->name, b->name); +} + -/* Check to see if our timezones are the same */ +/* Update the timezone entries: geo_location, current_location, + and the user-supplied custom locations */ static void -check_timezone_sync (void) { - gchar * label; - gboolean in_sync = FALSE; - - if (geo_timezone == NULL) { - in_sync = TRUE; - } +update_location_menu_items (void) +{ + if (get_greeter_mode ()) + return; - if (current_timezone == NULL) { - in_sync = TRUE; + g_assert (locations_separator != NULL); + g_assert (geo_location != NULL); + g_assert (current_location != NULL); + + GSList * visible_locations = NULL; + gchar ** locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S); + const guint location_count = locations ? g_strv_length(locations) : 0; + g_debug ("%s Found %u locations from %s", G_STRLOC, location_count, SETTINGS_LOCATIONS_S); + + const gboolean show_all = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); + + /* the timezones are in sync if they don't disagree with each other */ + const gboolean in_sync = !geo_timezone + || !current_timezone + || !g_strcmp0 (geo_timezone, current_timezone); + + /* decide whether or not to show the builtin locations */ + gboolean show_sep; + gboolean show_geo; + gboolean show_cur; + if (in_sync && !location_count) { + /* if the builtin timezones match, and there aren't any user user-specified locations, + then there's nothing interesting going on ... don't show anything */ + show_sep = FALSE; + show_geo = FALSE; + show_cur = FALSE; + } else { + /* if there are user-specified locations, show at least one builtin. + if the two builtins disagree, show both of them. */ + show_sep = TRUE; + show_geo = geo_timezone != NULL; + show_cur = (current_timezone != NULL) && (!in_sync || !show_geo); } - if (!in_sync && g_strcmp0(geo_timezone, current_timezone) == 0) { - in_sync = TRUE; + /* maybe add the builtin locations */ + if (show_geo) { + g_debug ("%s showing geo_timezone '%s'", G_STRLOC, geo_timezone); + set_current_timezone_label (geo_location, geo_timezone); + visible_locations = g_slist_prepend (visible_locations, name_and_offset_new_from_item (geo_location)); + } + if (show_cur) { + g_debug ("%s showing current_timezone '%s'", G_STRLOC, current_timezone); + set_current_timezone_label (current_location, current_timezone); + visible_locations = g_slist_prepend (visible_locations, name_and_offset_new_from_item (current_location)); } - if (in_sync) { - g_debug("Timezones in sync"); - } else { - g_debug("Timezones are different"); + /* remove the previous user-specified locations */ + while (location_menu_items != NULL) { + DbusmenuMenuitem * item = DBUSMENU_MENUITEM(location_menu_items->data); + location_menu_items = g_list_remove(location_menu_items, item); + dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item)); + g_object_unref(G_OBJECT(item)); } - gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); - - if (geo_location != NULL && current_location != NULL) { - g_debug("Got timezone %s", current_timezone); - g_debug("Got timezone %s", geo_timezone); - // Show neither current location nor geo location if both are the same - // however, we want to set their time and label accordingly - if (in_sync) { - if (current_timezone == NULL && geo_timezone == NULL) { - dbusmenu_menuitem_property_set_bool(locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - update_location_menu_items(); - return; - } - - dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); - dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); - - if (current_timezone != NULL) { - label = current_timezone; + /* add the user-specified locations */ + if (locations != NULL) { + gint i; + gint offset = dbusmenu_menuitem_get_position (current_location, root)+1; + for (i=0; idata); - location_menu_items = g_list_remove(location_menu_items, item); - dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item)); - g_object_unref(G_OBJECT(item)); - } - - const gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); - dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); - - gint i; - gint offset = dbusmenu_menuitem_get_position (current_location, root)+1; - for (i = 0; i < len; i++) { - // Iterate over configured places and add any which aren't already listed - if ((current_timezone == NULL || !g_str_has_prefix(locations[i], current_timezone)) && - (geo_timezone == NULL || !g_str_has_prefix(locations[i], geo_timezone))) { - DbusmenuMenuitem *item; - g_debug("Adding timezone in update_timezones %s", locations[i]); - item = dbusmenu_menuitem_new(); - dbusmenu_menuitem_property_set (item, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE); - set_timezone_label (item, locations[i]); - dbusmenu_menuitem_property_set_bool (item, TIMEZONE_MENUITEM_PROP_RADIO, FALSE); - dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); - dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, show); - dbusmenu_menuitem_child_add_position (root, item, offset++); - g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL); - location_menu_items = g_list_append(location_menu_items, item); - } - } - g_strfreev (locations); -} - // Authentication function static gchar * auth_func (ECal *ecal, @@ -1095,7 +1092,7 @@ static void show_locations_changed (void) { /* Re-calculate */ - check_timezone_sync(); + update_location_menu_items(); } static void @@ -1153,7 +1150,7 @@ build_menus (DbusmenuMenuitem * root) dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); dbusmenu_menuitem_child_append(root, current_location); - check_timezone_sync(); + update_location_menu_items(); g_signal_connect (conf, "changed::" SETTINGS_SHOW_LOCATIONS_S, G_CALLBACK (show_locations_changed), NULL); g_signal_connect (conf, "changed::" SETTINGS_LOCATIONS_S, G_CALLBACK (show_locations_changed), NULL); @@ -1294,7 +1291,7 @@ geo_address_cb (GeoclueAddress * address, int timestamp, GHashTable * addy_data, geo_timezone = g_strdup((gchar *)tz_hash); } - check_timezone_sync(); + update_location_menu_items(); return; } @@ -1391,7 +1388,7 @@ geo_client_invalid (GeoclueMasterClient * client, gpointer user_data) geo_timezone = NULL; } - check_timezone_sync(); + update_location_menu_items(); return; } @@ -1413,7 +1410,7 @@ geo_address_change (GeoclueMasterClient * client, gchar * a, gchar * b, gchar * geo_timezone = NULL; } - check_timezone_sync(); + update_location_menu_items(); return; } -- cgit v1.2.3 From 25994587f9c072e3243a44ca4786339085aa02e5 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 2 Mar 2012 14:13:18 -0600 Subject: use g_return_if_fail() instead of g_assert() in the new code --- src/datetime-service.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index 710b7d6..c7b2f69 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -169,9 +169,9 @@ update_location_menu_items (void) if (get_greeter_mode ()) return; - g_assert (locations_separator != NULL); - g_assert (geo_location != NULL); - g_assert (current_location != NULL); + g_return_if_fail (locations_separator != NULL); + g_return_if_fail (geo_location != NULL); + g_return_if_fail (current_location != NULL); GSList * visible_locations = NULL; gchar ** locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S); -- cgit v1.2.3 From ad8b10b4665dbab7a58f0272de9cb3009d0c74b0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 2 Mar 2012 15:02:31 -0600 Subject: simplify the code by removing special handling for geo_location and current_location, and adding them to the same 'locations' list that we use when pruning duplicates from the user-specified list of locations --- src/datetime-service.c | 229 ++++++++++++++++++++----------------------------- 1 file changed, 94 insertions(+), 135 deletions(-) diff --git a/src/datetime-service.c b/src/datetime-service.c index c7b2f69..c59c05b 100644 --- a/src/datetime-service.c +++ b/src/datetime-service.c @@ -82,9 +82,6 @@ static DbusmenuMenuitem * calendar = NULL; static DbusmenuMenuitem * settings = NULL; static DbusmenuMenuitem * events_separator = NULL; static DbusmenuMenuitem * locations_separator = NULL; -static DbusmenuMenuitem * geo_location = NULL; -static DbusmenuMenuitem * current_location = NULL; -//static DbusmenuMenuitem * ecal_location = NULL; static DbusmenuMenuitem * add_appointment = NULL; static GList * appointments = NULL; static GList * location_menu_items = NULL; @@ -110,112 +107,69 @@ struct comp_instance { ESource *source; }; -static void -set_current_timezone_label (DbusmenuMenuitem * mi, const gchar * location) -{ - gchar * name = get_current_zone_name (location); - - dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_NAME, name); - dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, location); - - g_free (name); -} - /** * A temp struct used by update_location_menu_items() for pruning duplicates. */ -struct NameAndOffset +struct TimeLocation { - gchar * name; gint32 offset; + gchar * zone; + gchar * name; }; static void -name_and_offset_free (struct NameAndOffset * nao) +time_location_free (struct TimeLocation * loc) { - g_free (nao->name); - g_free (nao); + g_free (loc->name); + g_free (loc->zone); + g_free (loc); } -static struct NameAndOffset* -name_and_offset_new (const char * zone, const char * name) +static struct TimeLocation* +time_location_new (const char * zone, const char * name) { - struct NameAndOffset * nao = g_new (struct NameAndOffset, 1); - nao->name = g_strdup (name); + struct TimeLocation * loc = g_new (struct TimeLocation, 1); GTimeZone * tz = g_time_zone_new (zone); - nao->offset = g_time_zone_get_offset (tz, 0); + loc->offset = g_time_zone_get_offset (tz, 0); + loc->zone = g_strdup (zone); + loc->name = g_strdup (name); g_time_zone_unref (tz); - return nao; -} -static struct NameAndOffset* -name_and_offset_new_from_item (DbusmenuMenuitem * mi) -{ - const char * zone = dbusmenu_menuitem_property_get (mi, TIMEZONE_MENUITEM_PROP_ZONE); - const char * name = dbusmenu_menuitem_property_get (mi, TIMEZONE_MENUITEM_PROP_NAME); - return name_and_offset_new (zone, name); + return loc; } static int -name_and_offset_compare (const struct NameAndOffset * a, const struct NameAndOffset * b) +time_location_compare (const struct TimeLocation * a, const struct TimeLocation * b) { + int ret; if (a->offset != b->offset) - return a->offset - b->offset; - return g_strcmp0 (a->name, b->name); + ret = a->offset - b->offset; + else + ret = g_strcmp0 (a->name, b->name); + g_debug ("%s comparing '%s' (%d) to '%s' (%d), returning %d", G_STRLOC, a->name, (int)a->offset, b->name, (int)b->offset, ret); + return ret; } +static GSList* +locations_add (GSList * locations, const char * zone, const char * name) +{ + struct TimeLocation * loc = time_location_new (zone, name); + if (g_slist_find_custom (locations, loc, (GCompareFunc)time_location_compare) == NULL) { + g_debug ("%s Adding zone '%s', name '%s'", G_STRLOC, zone, name); + locations = g_slist_prepend (locations, loc); + } else { + g_debug("%s Skipping duplicate zone '%s' name '%s'", G_STRLOC, zone, name); + time_location_free (loc); + } + return locations; +} -/* Update the timezone entries: geo_location, current_location, - and the user-supplied custom locations */ +/* Update the timezone entries */ static void update_location_menu_items (void) { - if (get_greeter_mode ()) + if (locations_separator == NULL) return; - g_return_if_fail (locations_separator != NULL); - g_return_if_fail (geo_location != NULL); - g_return_if_fail (current_location != NULL); - - GSList * visible_locations = NULL; - gchar ** locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S); - const guint location_count = locations ? g_strv_length(locations) : 0; - g_debug ("%s Found %u locations from %s", G_STRLOC, location_count, SETTINGS_LOCATIONS_S); - - const gboolean show_all = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S); - - /* the timezones are in sync if they don't disagree with each other */ - const gboolean in_sync = !geo_timezone - || !current_timezone - || !g_strcmp0 (geo_timezone, current_timezone); - - /* decide whether or not to show the builtin locations */ - gboolean show_sep; - gboolean show_geo; - gboolean show_cur; - if (in_sync && !location_count) { - /* if the builtin timezones match, and there aren't any user user-specified locations, - then there's nothing interesting going on ... don't show anything */ - show_sep = FALSE; - show_geo = FALSE; - show_cur = FALSE; - } else { - /* if there are user-specified locations, show at least one builtin. - if the two builtins disagree, show both of them. */ - show_sep = TRUE; - show_geo = geo_timezone != NULL; - show_cur = (current_timezone != NULL) && (!in_sync || !show_geo); - } - - /* maybe add the builtin locations */ - if (show_geo) { - g_debug ("%s showing geo_timezone '%s'", G_STRLOC, geo_timezone); - set_current_timezone_label (geo_location, geo_timezone); - visible_locations = g_slist_prepend (visible_locations, name_and_offset_new_from_item (geo_location)); - } - if (show_cur) { - g_debug ("%s showing current_timezone '%s'", G_STRLOC, current_timezone); - set_current_timezone_label (current_location, current_timezone); - visible_locations = g_slist_prepend (visible_locations, name_and_offset_new_from_item (current_location)); - } + GSList * locations = NULL; - /* remove the previous user-specified locations */ + /* remove the previous locations */ while (location_menu_items != NULL) { DbusmenuMenuitem * item = DBUSMENU_MENUITEM(location_menu_items->data); location_menu_items = g_list_remove(location_menu_items, item); @@ -223,50 +177,70 @@ update_location_menu_items (void) g_object_unref(G_OBJECT(item)); } - /* add the user-specified locations */ - if (locations != NULL) { + /*** + **** Build a list of locations to add: use geo_timezone, + **** current_timezone, and SETTINGS_LOCATIONS_S, but omit duplicates. + ***/ + + /* maybe add geo_timezone */ + if (geo_timezone != NULL) { + gchar * name = get_current_zone_name (geo_timezone); + locations = locations_add (locations, geo_timezone, name); + g_free (name); + } + + /* maybe add current_timezone */ + if (current_timezone != NULL) { + gchar * name = get_current_zone_name (current_timezone); + locations = locations_add (locations, current_timezone, name); + g_free (name); + } + + /* maybe add the user-specified custom locations */ + gchar ** user_locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S); + if (user_locations != NULL) { gint i; - gint offset = dbusmenu_menuitem_get_position (current_location, root)+1; + const guint location_count = g_strv_length(user_locations); + g_debug ("%s Found %u user-specified locations", G_STRLOC, location_count); for (i=0; inext) { + struct TimeLocation * loc = l->data; + g_debug("%s Adding location: zone '%s', name '%s'", G_STRLOC, loc->zone, loc->name); + DbusmenuMenuitem * item = dbusmenu_menuitem_new(); + dbusmenu_menuitem_property_set (item, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE); + dbusmenu_menuitem_property_set (item, TIMEZONE_MENUITEM_PROP_NAME, loc->name); + dbusmenu_menuitem_property_set (item, TIMEZONE_MENUITEM_PROP_ZONE, loc->zone); + dbusmenu_menuitem_property_set_bool (item, TIMEZONE_MENUITEM_PROP_RADIO, FALSE); + dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); + dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, show_locations); + dbusmenu_menuitem_child_add_position (root, item, offset++); + g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL); + location_menu_items = g_list_append (location_menu_items, item); + time_location_free (loc); + } + g_slist_free (locations); + locations = NULL; + + /* if there's at least one item being shown, show the separator too */ + dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show_locations && (location_menu_items!=NULL)); } /* Update the current timezone */ @@ -1135,21 +1109,6 @@ build_menus (DbusmenuMenuitem * root) dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); dbusmenu_menuitem_child_append(root, locations_separator); - geo_location = dbusmenu_menuitem_new(); - dbusmenu_menuitem_property_set (geo_location, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE); - set_current_timezone_label (geo_location, ""); - dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_ENABLED, FALSE); - dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE); - g_signal_connect(G_OBJECT(geo_location), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL); - dbusmenu_menuitem_child_append(root, geo_location); - - current_location = dbusmenu_menuitem_new(); - dbusmenu_menuitem_property_set (current_location, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE); - set_current_timezone_label (current_location, ""); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, FALSE); - dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE); - dbusmenu_menuitem_child_append(root, current_location); - update_location_menu_items(); g_signal_connect (conf, "changed::" SETTINGS_SHOW_LOCATIONS_S, G_CALLBACK (show_locations_changed), NULL); -- cgit v1.2.3