GNOME Bugzilla – Bug 700996
Lots of cleanup for realmd code
Last modified: 2013-05-27 05:40:00 UTC
Attaching several patches for cleanup of realmd code.
Created attachment 245290 [details] [review] account: Add debug output while joining a domain
Created attachment 245291 [details] [review] account: Correctly handle domains which are not joinable
Created attachment 245292 [details] [review] account: Connect to realmd asynchronously Even though realmd is pretty responsive for these calls, these are out of process calls so do them async
Created attachment 245293 [details] [review] account: Fix memory leak of discovered realms
Created attachment 245294 [details] [review] accounts: Complete realmd dicsovery on failure
Created attachment 245295 [details] [review] user-accounts: Tell realmd not to setup system management The Enterprise Login feature is meant to setup a individiual domain accounts for use with GNOME, and not manage the system. realmd now configures more than just logins, so tell realmd to hold back and not configure a system managed by the domain. https://bugzilla.gnome.org/show_bug.cgi?id=697910
Created attachment 245296 [details] [review] accounts: When enterprise account disabled, join as admin When an enterprise login user account is disabled, or needs to have its password change, just prompt for the administrator to do the join to the domain. This is exactly the same fall through as if the user does not have permission to join a machine to a domain. https://bugzilla.gnome.org/show_bug.cgi?id=699293
Created attachment 245297 [details] [review] accounts: Don't have domains twice in the drop down If we discover domains again, don't add them twice to the drop down and confuse the user. This is especially important if we receive two realms from realmd for the same domain for use with different clients (ie: sssd/winbind). We only want to offer the first choice https://bugzilla.gnome.org/show_bug.cgi?id=686397
Review of attachment 245290 [details] [review]: OK.
Review of attachment 245291 [details] [review]: ::: gnome-initial-setup/pages/account/um-realm-manager.c @@ +104,3 @@ + for (l = interfaces; l != NULL; l = g_list_next (l)) + on_interface_added (manager, object, l->data); + g_list_free_full (interfaces, g_object_unref); This seems unrelated?
Review of attachment 245292 [details] [review]: OK.
Review of attachment 245293 [details] [review]: OK.
Review of attachment 245294 [details] [review]: OK.
Review of attachment 245295 [details] [review]: OK.
Review of attachment 245296 [details] [review]: OK.
Review of attachment 245297 [details] [review]: ::: gnome-initial-setup/pages/account/gis-account-page.c @@ +799,3 @@ + /* + * Don't add a second realm if we already have one with this name. + * Sometimes realmd returns to realms for the same name, if it has "two" @@ +810,3 @@ + g_free (name); + if (match) { + g_debug ("ignoring duplicate realm: %s", realm_name); Isn't this leaking "common"? You should probably split this out into a separate helper method.
Attachment 245290 [details] pushed as d10d16e - account: Add debug output while joining a domain Attachment 245292 [details] pushed as 14d6abb - account: Connect to realmd asynchronously Attachment 245293 [details] pushed as ce39c2a - account: Fix memory leak of discovered realms Attachment 245294 [details] pushed as 5383989 - accounts: Complete realmd dicsovery on failure Attachment 245295 [details] pushed as 6ea3daa - user-accounts: Tell realmd not to setup system management Attachment 245296 [details] pushed as ed7af42 - accounts: When enterprise account disabled, join as admin
Created attachment 245349 [details] [review] account: Set timeout properly on all realm interface proxies
Created attachment 245350 [details] [review] account: Correctly handle domains which are not joinable Updated. Split into two patches.
Created attachment 245351 [details] [review] accounts: Don't have domains twice in the drop down Updated, fixed typo and leak. Didn't add helper method since we only do um_realm_object_get_common() + um_realm_common_get_name() combo once.
Review of attachment 245351 [details] [review]: ::: gnome-initial-setup/pages/account/gis-account-page.c @@ +811,3 @@ + if (match) { + g_debug ("ignoring duplicate realm: %s", realm_name); + g_object_unref (common); Yeah, this is still a bit error prone. Either use a "goto out;" or split this out into a separate "model_contains_realm"
Review of attachment 245349 [details] [review]: OK.
Review of attachment 245350 [details] [review]: OK.
Attachment 245349 [details] pushed as 3a77768 - account: Set timeout properly on all realm interface proxies Attachment 245350 [details] pushed as ca14cf2 - account: Correctly handle domains which are not joinable
Created attachment 245358 [details] [review] accounts: Don't have domains twice in the drop down Okay, changed to use helper functions. Hopefully that addresses the issue you raised.
Review of attachment 245358 [details] [review]: OK.
Attachment 245358 [details] pushed as 939eaa5 - accounts: Don't have domains twice in the drop down