Merge lp:~kvalo/indicator-network/gdbus-cellular-crash into lp:~indicator-applet-developers/indicator-network/indicator-network

[Kalle Valo]

[Conor Curran (cjcurran)]

I thought 'goto's were frowned upon, just stylistic. +1

review: Approve

[Kalle Valo (kvalo)]

Conor Curran <email address hidden> writes:

> Review: Approve

Thanks.

> I thought 'goto's were frowned upon, just stylistic. +1

Not everywhere. For example, when exiting a function you can save some
code by using goto. One can consider "goto out" as a sophisticated form
of "return" :)

--
Kalle Valo

=== modified file 'src/backend/ofono-manager.c'
--- src/backend/ofono-manager.c	2010-11-30 10:52:15 +0000
+++ src/backend/ofono-manager.c	2010-12-09 13:14:44 +0000
@@ -153,6 +153,17 @@
     /* sim requires pin now, ask from ui */
     ask_pin(self, proxy);
   }
+
+  g_variant_unref(parameters);
+}
+
+static void update_sim_property(OfonoManager *self, GDBusProxy *proxy,
+				const gchar *property, GVariant *value)
+{
+  if (g_strcmp0(property, OFONO_PROPERTY_PIN_REQUIRED) == 0)
+    update_pin_required(self, proxy, value);
+  else
+    g_variant_unref(value);
 }
 
 /* callee owns parameters variant */
@@ -162,10 +173,10 @@
   GVariant *value;
   gchar *name;
 
-  g_variant_get(parameters, "{sv}", &name, &value);
+  g_variant_get(parameters, "(sv)", &name, &value);
 
-  if (g_strcmp0(name, OFONO_PROPERTY_PIN_REQUIRED) == 0)
-    update_pin_required(self, proxy, value);
+  /* callee takes the ownership of the value */
+  update_sim_property(self, proxy, name, value);
 
   g_free(name);
 }
@@ -178,17 +189,20 @@
   GVariant *result, *value, *dict;
   GVariantIter iter, dict_iter;
   GError *error = NULL;
+  gchar *name;
 
   result = g_dbus_proxy_call_finish(proxy, res, &error);
 
   if (error != NULL) {
-    g_warning("Failed to get ofono proxy: %s", error->message);
+    g_warning("Failed to get sim properties from %s: %s",
+	      g_dbus_proxy_get_object_path(proxy), error->message);
     g_error_free(error);
     goto out;
   }
 
   if (result == NULL) {
-    g_warning("Failed to get ofono proxy, but no errors");
+    g_warning("Failed to get sim properties from %s, but no errors",
+	      g_dbus_proxy_get_object_path(proxy));
     goto out;
   }
 
@@ -200,10 +214,11 @@
 
   g_variant_iter_init(&dict_iter, dict);
 
-  while ((value = g_variant_iter_next_value(&dict_iter)) != NULL) {
-    sim_property_changed(self, proxy, value);
+  while (g_variant_iter_next(&dict_iter, "{sv}", &name, &value)) {
+    /* callee owns the variant now */
+    update_sim_property(self, proxy, name, value);
 
-    g_variant_unref(value);
+    g_free(name);
   }
 
   g_variant_unref(result);
@@ -312,6 +327,7 @@
   path = g_dbus_proxy_get_object_path(proxy);
 
   g_variant_iter_init(&iter, variant);
+  found = FALSE;
 
   while (g_variant_iter_next(&iter, "s", &interface)) {
     if (g_strcmp0(interface, OFONO_SIM_INTERFACE) == 0)
@@ -336,7 +352,7 @@
   g_variant_get(parameters, "(sv)", &name, &value);
 
   if (g_strcmp0(name, OFONO_PROPERTY_INTERFACES) == 0)
-    update_modem_interfaces(self, proxy, parameters);
+    update_modem_interfaces(self, proxy, value);
 
   g_variant_unref(value);
   g_free(name);
@@ -355,13 +371,15 @@
   result = g_dbus_proxy_call_finish(proxy, res, &error);
 
   if (error != NULL) {
-    g_warning("Failed to get ofono proxy: %s", error->message);
+    g_warning("Failed to get modem properties from %s: %s",
+	      g_dbus_proxy_get_object_path(proxy), error->message);
     g_error_free(error);
     goto out;
   }
 
   if (result == NULL) {
-    g_warning("Failed to get ofono proxy, but no errors");
+    g_warning("Failed to get modem properties from %s, but no errors",
+	      g_dbus_proxy_get_object_path(proxy));
     goto out;
   }