fixed number of failing test cases, memory leaks and invalid reads

diff --git a/libsignon-glib/signon-auth-service.c b/libsignon-glib/signon-auth-service.c
index 58d61d06576f81d259cc0c35bfafca1e59e16329..599080cd99b5dae7ad9609e96b65de80a5893050 100644 (file)
--- a/libsignon-glib/signon-auth-service.c
+++ b/libsignon-glib/signon-auth-service.c
@@ -152,8 +152,7 @@ auth_query_methods_cb (GObject *object, GAsyncResult *res,
     (data->cb)
         (data->service, value, error, data->userdata);
 
-    if (error)
-        g_error_free (error);
+    g_clear_error (&error);
     g_slice_free (MethodCbData, data);
 }
 

diff --git a/libsignon-glib/signon-auth-session.c b/libsignon-glib/signon-auth-session.c
index d3b4b6ca1f8bebae7fb220bb223b8cfc12dd46f9..83f4407b689f428f3f67fdd5d82f4a0f16776059 100644 (file)
--- a/libsignon-glib/signon-auth-session.c
+++ b/libsignon-glib/signon-auth-session.c
@@ -752,12 +752,12 @@ auth_session_query_mechanisms_reply (GObject *object, GAsyncResult *res,
                                                              &mechanisms,
                                                              res,
                                                              &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
-    (cb_data->cb) (cb_data->self, mechanisms, error, cb_data->user_data);
-
-    if (error)
-        g_error_free (error);
+    if (SIGNON_IS_NOT_CANCELLED (error))
+    {
+        (cb_data->cb) (cb_data->self, mechanisms, error, cb_data->user_data);
+    }
 
+    g_clear_error (&error);
     g_slice_free (AuthSessionQueryAvailableMechanismsCbData, cb_data);
 }
 

diff --git a/libsignon-glib/signon-identity.c b/libsignon-glib/signon-identity.c
index 40ae146f960ae082ecb44a13bcea4dd410a94e65..808d3698efbe3094982dbd9989c985e3767f8fa8 100644 (file)
--- a/libsignon-glib/signon-identity.c
+++ b/libsignon-glib/signon-identity.c
@@ -537,9 +537,12 @@ identity_new_cb (GObject *object, GAsyncResult *res,
                                                         &object_path,
                                                         res,
                                                         &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
-    identity_registered (identity, object_path, NULL, error);
-    g_free (object_path);
+    if (SIGNON_IS_NOT_CANCELLED (error))
+    {
+        identity_registered (identity, object_path, NULL, error);
+    }
+    if (object_path) g_free (object_path);
+    g_clear_error (&error);
 }
 
 static void
@@ -560,9 +563,12 @@ identity_new_from_db_cb (GObject *object, GAsyncResult *res,
                                                &identity_data,
                                                res,
                                                &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
-    identity_registered (identity, object_path, identity_data, error);
-    g_free (object_path);
+    if (SIGNON_IS_NOT_CANCELLED (error))
+    {
+        identity_registered (identity, object_path, identity_data, error);
+    }
+    if (object_path) g_free (object_path);
+    g_clear_error (&error);
 }
 
 static void
@@ -880,7 +886,6 @@ identity_store_credentials_reply (GObject *object, GAsyncResult *res,
     SignonIdentityPrivate *priv = cb_data->self->priv;
 
     sso_identity_call_store_finish (proxy, &id, res, &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
 
     if (error == NULL)
     {
@@ -896,7 +901,7 @@ identity_store_credentials_reply (GObject *object, GAsyncResult *res,
         priv->removed = FALSE;
     }
 
-    if (cb_data->cb)
+    if (SIGNON_IS_NOT_CANCELLED (error) && cb_data->cb)
     {
         (cb_data->cb) (cb_data->self, id, error, cb_data->user_data);
     }
@@ -918,9 +923,8 @@ identity_verify_reply (GObject *object, GAsyncResult *res,
     g_return_if_fail (cb_data->self != NULL);
 
     sso_identity_call_verify_secret_finish (proxy, &valid, res, &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
 
-    if (cb_data->cb)
+    if (SIGNON_IS_NOT_CANCELLED (error) && cb_data->cb)
     {
         (cb_data->cb) (cb_data->self, valid, error, cb_data->user_data);
     }
@@ -1129,9 +1133,8 @@ identity_signout_reply (GObject *object, GAsyncResult *res,
     g_return_if_fail (cb_data->self->priv != NULL);
 
     sso_identity_call_sign_out_finish (proxy, &result, res, &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
 
-    if (cb_data->cb)
+    if (SIGNON_IS_NOT_CANCELLED (error) && cb_data->cb)
     {
         (cb_data->cb) (cb_data->self, error, cb_data->user_data);
     }
@@ -1153,9 +1156,8 @@ identity_removed_reply (GObject *object, GAsyncResult *res,
     g_return_if_fail (cb_data->self->priv != NULL);
 
     sso_identity_call_remove_finish (proxy, res, &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
 
-    if (cb_data->cb)
+    if (SIGNON_IS_NOT_CANCELLED (error) && cb_data->cb)
     {
         (cb_data->cb) (cb_data->self, error, cb_data->user_data);
     }
@@ -1182,13 +1184,15 @@ identity_info_reply(GObject *object, GAsyncResult *res,
     SignonIdentityPrivate *priv = cb_data->self->priv;
 
     sso_identity_call_get_info_finish (proxy, &identity_data, res, &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
-    priv->identity_info =
-        signon_identity_info_new_from_variant (identity_data);
+
     if (identity_data != NULL)
+    {
+        priv->identity_info =
+                signon_identity_info_new_from_variant (identity_data);
         g_variant_unref (identity_data);
+    }
 
-    if (cb_data->cb)
+    if (SIGNON_IS_NOT_CANCELLED (error) && cb_data->cb)
     {
         (cb_data->cb) (cb_data->self, priv->identity_info, error, cb_data->user_data);
     }
@@ -1534,20 +1538,21 @@ identity_get_auth_session_reply (GObject *object, GAsyncResult *res,
                                                &object_path,
                                                res,
                                                &error);
-    SIGNON_RETURN_IF_CANCELLED (error);
 
     IdentitySessionCbData *cb_data = (IdentitySessionCbData *) userdata;
     g_return_if_fail (cb_data != NULL);
     g_return_if_fail (cb_data->cb != NULL);
 
-    (cb_data->cb) (cb_data->session,
-                   error,
-                   g_dbus_proxy_get_connection ((GDBusProxy *)proxy),
-                   g_dbus_proxy_get_name ((GDBusProxy *)proxy),
-                   object_path);
-
+    if (SIGNON_IS_NOT_CANCELLED (error))
+    {
+        (cb_data->cb) (cb_data->session,
+                error,
+                g_dbus_proxy_get_connection ((GDBusProxy *)proxy),
+                g_dbus_proxy_get_name ((GDBusProxy *)proxy),
+                object_path);
+    }
     g_slice_free (IdentitySessionCbData, cb_data);
-    g_free (object_path);
+    if (object_path) g_free (object_path);
     g_clear_error (&error);
 }
 
@@ -1566,17 +1571,18 @@ identity_session_ready_cb(gpointer object, const GError *error, gpointer user_da
 
     IdentitySessionCbData *cb_data = operation_data->cb_data;
     g_return_if_fail (cb_data != NULL);
+    g_return_if_fail (cb_data->cb != NULL);
 
-    if (priv->removed == TRUE)
+    if (error)
+    {
+        (cb_data->cb) (cb_data->session, (GError *)error, NULL, NULL, NULL);
+    }
+    else if (priv->removed == TRUE)
     {
         GError *new_error = g_error_new (signon_error_quark(),
                                          SIGNON_ERROR_IDENTITY_NOT_FOUND,
                                          "Already removed from database.");
-        if (cb_data->cb)
-        {
-            (cb_data->cb) (cb_data->session, new_error, NULL, NULL, NULL);
-        }
-
+        (cb_data->cb) (cb_data->session, new_error, NULL, NULL, NULL);
         g_error_free (new_error);
     }
     else

diff --git a/libsignon-glib/signon-utils.h b/libsignon-glib/signon-utils.h
index b3cf0924d61768e2844bc2a4f36dca7d20d476fe..32a404b0389a3b65e40dcefd1d571f1046699545 100644 (file)
--- a/libsignon-glib/signon-utils.h
+++ b/libsignon-glib/signon-utils.h
@@ -27,14 +27,10 @@
 
 #include <glib-object.h>
 
-#define SIGNON_RETURN_IF_CANCELLED(error) \
-    if (error != NULL && \
-        error->domain == G_IO_ERROR && \
-        error->code == G_IO_ERROR_CANCELLED) \
-    { \
-        g_error_free (error); \
-        return; \
-    }
+#define SIGNON_IS_NOT_CANCELLED(error) \
+        (error == NULL || \
+        error->domain != G_IO_ERROR || \
+        error->code != G_IO_ERROR_CANCELLED)
 
 G_GNUC_INTERNAL
 GValue *signon_gvalue_new (GType type);

diff --git a/tests/check_signon.c b/tests/check_signon.c
index 85de53b4e2e4b85130ce27c40a027784c38d23d5..6df9cf71238586d46a66e9ac055309ce81be42eb 100644 (file)
--- a/tests/check_signon.c
+++ b/tests/check_signon.c
@@ -50,7 +50,6 @@ static GMainLoop *main_loop = NULL;
 static SignonIdentity *identity = NULL;
 static SignonAuthService *auth_service = NULL;
 static gboolean id_destroyed = FALSE;
-static gboolean auth_sess_destroyed = FALSE;
 
 #define SIGNOND_IDLE_TIMEOUT (5 + 2)
 
@@ -101,6 +100,61 @@ _teardown ()
     }
 }
 
+static void
+new_identity_store_credentials_cb(
+        SignonIdentity *self,
+        guint32 id,
+        const GError *error,
+        gpointer user_data)
+{
+    gint *new_id = user_data;
+
+    if(error)
+    {
+        g_warning ("%s %d: %s", G_STRFUNC, __LINE__, error->message);
+        fail();
+    }
+
+    fail_unless (id > 0);
+
+    *new_id = id;
+
+    _stop_mainloop ();
+}
+
+static guint
+new_identity()
+{
+    SignonIdentity *idty;
+    GHashTable *methods;
+    guint id = 0;
+
+    idty = signon_identity_new (NULL);
+    fail_unless (SIGNON_IS_IDENTITY (idty));
+    methods = g_hash_table_new (g_str_hash, g_str_equal);
+    g_hash_table_insert (methods, "ssotest", ssotest_mechanisms);
+    signon_identity_store_credentials_with_args (idty,
+                                                 "James Bond",
+                                                 "007",
+                                                 TRUE,
+                                                 methods,
+                                                 "MI-6",
+                                                 NULL,
+                                                 NULL,
+                                                 NULL,
+                                                 0,
+                                                 new_identity_store_credentials_cb,
+                                                 &id);
+    g_hash_table_destroy (methods);
+
+    if (id == 0)
+        _run_mainloop ();
+
+    g_object_unref (idty);
+
+    return id;
+
+}
 
 START_TEST(test_init)
 {
@@ -120,23 +174,26 @@ signon_query_methods_cb (SignonAuthService *auth_service, gchar **methods,
     {
         g_warning ("%s: %s", G_STRFUNC, error->message);
         _stop_mainloop ();
+        if (methods) g_strfreev (methods);
         fail();
     }
 
     gboolean has_ssotest = FALSE;
+    gchar **pmethods = methods;
 
     fail_unless (g_strcmp0 (user_data, "Hello") == 0, "Got wrong string");
     fail_unless (methods != NULL, "The methods does not exist");
 
-    while (*methods)
+    while (*pmethods)
     {
-        if (g_strcmp0 (*methods, "ssotest") == 0)
+        if (g_strcmp0 (*pmethods, "ssotest") == 0)
         {
             has_ssotest = TRUE;
             break;
         }
-        methods++;
+        pmethods++;
     }
+    g_strfreev (methods);
     fail_unless (has_ssotest, "ssotest method does not exist");
 
     _stop_mainloop ();
@@ -163,6 +220,7 @@ signon_query_mechanisms_cb (SignonAuthService *auth_service, gchar *method,
     if (error)
     {
         g_warning ("%s: %s", G_STRFUNC, error->message);
+        if (mechanisms) g_strfreev (mechanisms);
         _stop_mainloop ();
         fail();
     }
@@ -170,24 +228,25 @@ signon_query_mechanisms_cb (SignonAuthService *auth_service, gchar *method,
     gboolean has_mech1 = FALSE;
     gboolean has_mech2 = FALSE;
     gboolean has_mech3 = FALSE;
+    gchar **pmechanisms = mechanisms;
 
     fail_unless (strcmp (user_data, "Hello") == 0, "Got wrong string");
     fail_unless (mechanisms != NULL, "The mechanisms does not exist");
 
-    while (*mechanisms)
+    while (*pmechanisms)
     {
-        if (g_strcmp0 (*mechanisms, "mech1") == 0)
+        if (g_strcmp0 (*pmechanisms, "mech1") == 0)
             has_mech1 = TRUE;
 
-        if (g_strcmp0 (*mechanisms, "mech2") == 0)
+        if (g_strcmp0 (*pmechanisms, "mech2") == 0)
             has_mech2 = TRUE;
 
-        if (g_strcmp0 (*mechanisms, "mech3") == 0)
+        if (g_strcmp0 (*pmechanisms, "mech3") == 0)
             has_mech3 = TRUE;
 
-        mechanisms++;
+        pmechanisms++;
     }
-
+    g_strfreev (mechanisms);
     fail_unless (has_mech1, "mech1 mechanism does not exist");
     fail_unless (has_mech2, "mech2 mechanism does not exist");
     fail_unless (has_mech3, "mech3 mechanism does not exist");
@@ -205,6 +264,7 @@ signon_query_mechanisms_cb_fail (SignonAuthService *auth_service,
     fail_unless (mechanisms == NULL);
     fail_unless (error->domain == SIGNON_ERROR);
     fail_unless (error->code == SIGNON_ERROR_METHOD_NOT_KNOWN);
+    if (mechanisms) g_strfreev (mechanisms);
     _stop_mainloop ();
 }
 
@@ -320,7 +380,8 @@ START_TEST(test_auth_session_query_mechanisms)
     _run_mainloop ();
 
     g_free(patterns[0]);
-    g_object_unref(idty);
+    g_object_unref (auth_session);
+    g_object_unref (idty);
 
 }
 END_TEST
@@ -373,7 +434,8 @@ START_TEST(test_auth_session_query_mechanisms_nonexisting)
     g_free(patterns[0]);
     g_free(patterns[1]);
     g_free(patterns[2]);
-    g_object_unref(idty);
+    g_object_unref (auth_session);
+    g_object_unref (idty);
 
 }
 END_TEST
@@ -429,12 +491,14 @@ _on_identity_destroyed (gpointer data, GObject *obj)
 static void
 _on_auth_session_destroyed (gpointer data, GObject *obj)
 {
-    auth_sess_destroyed = TRUE;
+    gboolean *is_destroyed = (gboolean *)data;
+    *is_destroyed = TRUE;
 }
 
 START_TEST(test_auth_session_creation)
 {
     GError *err = NULL;
+    gboolean auth_sess_destroyed = FALSE;
 
     g_debug("%s", G_STRFUNC);
     SignonIdentity *idty = signon_identity_new(NULL);
@@ -447,9 +511,9 @@ START_TEST(test_auth_session_creation)
     fail_unless (auth_session != NULL, "Cannot create AuthSession object");
 
     id_destroyed = FALSE;
-    auth_sess_destroyed = FALSE;
     g_object_weak_ref (G_OBJECT (idty), _on_identity_destroyed, NULL);
-    g_object_weak_ref (G_OBJECT (auth_session), _on_auth_session_destroyed, NULL);
+    g_object_weak_ref (G_OBJECT (auth_session), _on_auth_session_destroyed,
+            &auth_sess_destroyed);
 
     g_object_unref (idty);
     fail_unless (id_destroyed == FALSE, "Identity must stay untill all its session are not destroyed");
@@ -579,7 +643,12 @@ START_TEST(test_auth_session_process_failure)
 
     g_debug("%s", G_STRFUNC);
 
-    idty = signon_identity_new_from_db (1, NULL);
+    guint id = new_identity();
+
+    fail_unless (id != 0);
+
+    idty = signon_identity_new_from_db (id, NULL);
+
     fail_unless (idty != NULL, "Cannot create Identity object");
     auth_session = signon_auth_session_new (G_OBJECT (idty),
                                             "ssotest",
@@ -728,58 +797,6 @@ static GHashTable *create_methods_hashtable()
     return methods;
 }
 
-static void new_identity_store_credentials_cb(SignonIdentity *self,
-                                              guint32 id,
-                                              const GError *error,
-                                              gpointer user_data)
-{
-    gint *new_id = user_data;
-
-    if(error)
-    {
-        g_warning ("%s %d: %s", G_STRFUNC, __LINE__, error->message);
-        fail();
-    }
-
-    fail_unless (id > 0);
-
-    *new_id = id;
-
-    _stop_mainloop ();
-}
-
-static guint
-new_identity()
-{
-    SignonIdentity *idty;
-    GHashTable *methods;
-    guint id = 0;
-
-    idty = signon_identity_new (NULL);
-    fail_unless (SIGNON_IS_IDENTITY (idty));
-    methods = g_hash_table_new (g_str_hash, g_str_equal);
-    g_hash_table_insert (methods, "ssotest", ssotest_mechanisms);
-    signon_identity_store_credentials_with_args (idty,
-                                                 "James Bond",
-                                                 "007",
-                                                 TRUE,
-                                                 methods,
-                                                 "MI-6",
-                                                 NULL,
-                                                 NULL,
-                                                 NULL,
-                                                 0,
-                                                 new_identity_store_credentials_cb,
-                                                 &id);
-    g_hash_table_destroy (methods);
-
-    if (id == 0)
-        _run_mainloop ();
-
-    return id;
-
-}
-
 static gboolean
 identity_registeration_timeout_cb (gpointer data)
 {
@@ -1166,6 +1183,7 @@ static void identity_signout_signal_cb (gpointer instance, gpointer user_data)
 
 START_TEST(test_signout_identity)
 {
+    gboolean as1_destroyed = FALSE, as2_destroyed = FALSE;
     g_debug("%s", G_STRFUNC);
     SignonIdentity *idty = signon_identity_new (NULL);
     fail_unless (idty != NULL);
@@ -1210,12 +1228,16 @@ START_TEST(test_signout_identity)
     g_signal_connect (idty2, "signout",
                       G_CALLBACK(identity_signout_signal_cb), &counter);
 
+    g_object_weak_ref (G_OBJECT (as1), _on_auth_session_destroyed,
+            &as1_destroyed);
+    g_object_weak_ref (G_OBJECT (as2), _on_auth_session_destroyed,
+            &as2_destroyed);
     signon_identity_signout (idty, identity_signout_cb, NULL);
     _run_mainloop ();
 
     fail_unless (counter == 2, "Lost some of SIGNOUT signals");
-    fail_if (SIGNON_IS_AUTH_SESSION (as1), "Authsession1 was not destroyed after signout");
-    fail_if (SIGNON_IS_AUTH_SESSION (as2), "Authsession2 was not destroyed after signout");
+    fail_if (as1_destroyed == FALSE, "Authsession1 was not destroyed after signout");
+    fail_if (as2_destroyed == FALSE, "Authsession2 was not destroyed after signout");
 
     g_object_unref (idty);
     g_object_unref (idty2);
@@ -1354,7 +1376,10 @@ START_TEST(test_regression_unref)
 
     g_debug ("%s", G_STRFUNC);
 
-    idty = signon_identity_new_from_db (1, NULL);
+    guint id = new_identity();
+    fail_unless (id != 0);
+    idty = signon_identity_new_from_db (id, NULL);
+
     fail_unless (idty != NULL);
     auth_session = signon_auth_session_new (G_OBJECT (idty), "ssotest",
             &error);
@@ -1395,6 +1420,7 @@ signon_suite(void)
     tcase_set_timeout(tc_core, 1080);
     tcase_add_test (tc_core, test_init);
     tcase_add_test (tc_core, test_query_methods);
+
     tcase_add_test (tc_core, test_query_mechanisms);
     tcase_add_test (tc_core, test_get_existing_identity);
     tcase_add_test (tc_core, test_get_nonexisting_identity);