From: Imran Zaman Date: Fri, 17 May 2013 15:17:00 +0000 (+0300) Subject: fixed number of failing test cases, memory leaks and invalid reads X-Git-Tag: upstream/2.4.0^2~104 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=96051679b87c143a86b17246f7004ced3e90fd41;p=platform%2Fupstream%2Flibgsignon-glib.git 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 58d61d0..599080c 100644 --- 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 d3b4b6c..83f4407 100644 --- 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 40ae146..808d369 100644 --- 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 b3cf092..32a404b 100644 --- a/libsignon-glib/signon-utils.h +++ b/libsignon-glib/signon-utils.h @@ -27,14 +27,10 @@ #include -#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 85de53b..6df9cf7 100644 --- 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);