Tidy up the GError logic in dup_value, and document the value+error behaviour

2 files changed, 19 insertions, 6 deletions

diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index 3bd3b753..7ca0feff 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -235,6 +235,10 @@ mcd_storage_dup_string (McdStorage *storage,
  * with tp_g_value_slice_free() or g_slice_free() depending on whether the
  * the value itself should be freed (the former frees everything, the latter
  * only the #GValue container.
+ *
+ * If @error is set, but a non-%NULL value was returned, this indicates
+ * that no value for the @key was found for @account, and the default
+ * value for @type has been returned.
  **/
 GValue *
 mcd_storage_dup_value (McdStorage *storage,
diff --git a/src/plugin-account.c b/src/plugin-account.c
index ded43084..57380415 100644
--- a/src/plugin-account.c
+++ b/src/plugin-account.c
@@ -502,12 +502,12 @@ _storage_dup_value (McdStorage *storage,
         break;
 
       case G_TYPE_BOOLEAN:
-        v_bool = g_key_file_get_boolean (keyfile, account, key, NULL);
+        v_bool = g_key_file_get_boolean (keyfile, account, key, error);
         value = tp_g_value_slice_new_boolean (v_bool);
         break;
 
       case G_TYPE_DOUBLE:
-        v_double = g_key_file_get_double (keyfile, account, key, NULL);
+        v_double = g_key_file_get_double (keyfile, account, key, error);
         value = tp_g_value_slice_new_double (v_double);
         break;
 
@@ -521,9 +521,15 @@ _storage_dup_value (McdStorage *storage,
           }
         else if (type == DBUS_TYPE_G_OBJECT_PATH)
           {
-            v_string = g_key_file_get_string (keyfile, account, key, error);
+            v_string = g_key_file_get_string (keyfile, account, key, NULL);
 
-            if (!tp_dbus_check_valid_object_path (v_string, NULL))
+            if (v_string == NULL)
+              {
+                g_set_error (error, MCD_ACCOUNT_ERROR,
+                    MCD_ACCOUNT_ERROR_GET_PARAMETER,
+                    "Invalid object path NULL");
+              }
+            else if (!tp_dbus_check_valid_object_path (v_string, NULL))
               {
                 g_set_error (error, MCD_ACCOUNT_ERROR,
                     MCD_ACCOUNT_ERROR_GET_PARAMETER,
@@ -542,8 +548,11 @@ _storage_dup_value (McdStorage *storage,
           }
     }
 
-  if (value != NULL)
-    g_clear_error (error);
+  /* This can return a non-NULL GValue * _and_ a non-NULL GError *,      *
+   * indicating a value was not found and the default for that type      *
+   * (eg 0 for integers) has been returned - this matches the behaviour  *
+   * of the old code that this function replaces. If changing this, make *
+   * sure all our callers are suitable updated                           */
 
   return value;
 }