From 68c54502cb9a29ee0e3ebf640a1595008c7b89b2 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Wed, 13 Jun 2012 11:42:28 +0200 Subject: [PATCH 1/2] Refactor reloading of users from multiple sources * Don't remove users from the list when reloading /etc/passwd if they're elsewhere. * Treat files listed in /var/lib/AccountsService/users as 'cached' users https://bugs.freedesktop.org/show_bug.cgi?id=50770 --- src/daemon.c | 323 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 205 insertions(+), 118 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 12f189e..7754e80 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -202,25 +202,39 @@ daemon_local_user_is_excluded (Daemon *daemon, const gchar *username, const gcha return ret; } -static void -reload_wtmp_history (Daemon *daemon) +static GHashTable * +users_hash_table_new (void) { + return g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_object_unref); +} + #ifdef HAVE_UTMPX_H - struct utmpx *wtmp_entry; + +static struct passwd * +entry_generator_wtmp (GHashTable *users, + gpointer *state) +{ GHashTable *login_frequency_hash; + struct utmpx *wtmp_entry; GHashTableIter iter; gpointer key, value; + struct passwd *pwent; + /* First iteration */ + if (*state == NULL) { #ifdef UTXDB_LOG - if (setutxdb (UTXDB_LOG, NULL) != 0) - return; + if (setutxdb (UTXDB_LOG, NULL) != 0) + return NULL; #else - utmpxname (_PATH_WTMPX); - setutxent (); + utmpxname (_PATH_WTMPX); + setutxent (); #endif + *state = g_hash_table_new (g_str_hash, g_str_equal); + } - login_frequency_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL); - + /* Next iterations */ + login_frequency_hash = *state; while ((wtmp_entry = getutxent ())) { if (wtmp_entry->ut_type != USER_PROCESS) continue; @@ -228,12 +242,9 @@ reload_wtmp_history (Daemon *daemon) if (wtmp_entry->ut_user[0] == 0) continue; - if (daemon_local_user_is_excluded (daemon, - wtmp_entry->ut_user, - NULL)) { - g_debug ("excluding user '%s'", wtmp_entry->ut_user); + pwent = getpwnam (wtmp_entry->ut_user); + if (pwent == NULL) continue; - } if (!g_hash_table_lookup_extended (login_frequency_hash, wtmp_entry->ut_user, @@ -251,81 +262,171 @@ reload_wtmp_history (Daemon *daemon) GUINT_TO_POINTER (frequency)); } + return pwent; } + + /* Complete */ endutxent (); g_hash_table_iter_init (&iter, login_frequency_hash); while (g_hash_table_iter_next (&iter, &key, &value)) { User *user; - char *username = (char *) key; guint64 frequency = (guint64) GPOINTER_TO_UINT (value); - user = daemon_local_find_user_by_name (daemon, username); - if (user == NULL) { - g_debug ("unable to lookup user '%s'", username); + user = g_hash_table_lookup (users, key); + if (user == NULL) continue; - } g_object_set (user, "login-frequency", frequency, NULL); } g_hash_table_foreach (login_frequency_hash, (GHFunc) g_free, NULL); g_hash_table_unref (login_frequency_hash); + *state = NULL; + return NULL; +} + #endif /* HAVE_UTMPX_H */ + +#ifdef HAVE_FGETPWENT + +static struct passwd * +entry_generator_fgetpwent (GHashTable *users, + gpointer *state) +{ + struct passwd *pwent; + FILE *fp; + + /* First iteration */ + if (*state == NULL) { + *state = fp = fopen (PATH_PASSWD, "r"); + if (fp == NULL) { + g_warning ("Unable to open %s: %s", PATH_PASSWD, g_strerror (errno)); + return NULL; + } + } + + /* Later iterations */ + fp = *state; + pwent = fgetpwent (fp); + if (pwent != NULL) + return pwent; + + /* Complete? */ + fclose (fp); + *state = NULL; + return NULL; } -static void -listify_hash_values_hfunc (gpointer key, - gpointer value, - gpointer user_data) +#else /* !HAVE_FGETPWENT */ + +static struct passwd * +entry_generator_getpwent (GHashTable *users, + gpointer *state) { - GSList **list = user_data; + struct passwd *pwent; - *list = g_slist_prepend (*list, value); + /* First iteration */ + if (*state == NULL) { + setpwent(); + *state = GINT_TO_POINTER (1); + } + + /* Iterations */ + pwent = getpwent (); + if (pwent != NULL) + return pwent; + + /* Complete */ + *state = NULL; + return NULL; } -static gint -compare_user_name (gconstpointer a, gconstpointer b) +#endif /* !HAVE_FGETPWENT */ + +static struct passwd * +entry_generator_cachedir (GHashTable *users, + gpointer *state) { - User *user = (User *)a; - const gchar *name = b; + struct passwd *pwent; + const gchar *name; + GError *error = NULL; + gchar *filename; + gboolean regular; + GHashTableIter iter; + GKeyFile *key_file; + User *user; + GDir *dir; + + /* First iteration */ + if (*state == NULL) { + *state = dir = g_dir_open (USERDIR, 0, &error); + if (error != NULL) { + if (!g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT)) + g_warning ("couldn't list user cache directory: %s", USERDIR); + g_error_free (error); + return NULL; + } + } + + /* + * Use names of files of regular type to lookup information + * about each user. Loop until we find something valid. + */ + dir = *state; + while (TRUE) { + name = g_dir_read_name (dir); + if (name == NULL) + break; + + /* Only load files in this directory */ + filename = g_build_filename (USERDIR, name, NULL); + regular = g_file_test (filename, G_FILE_TEST_IS_REGULAR); + g_free (filename); + + if (regular) { + pwent = getpwnam (name); + if (pwent == NULL) + g_debug ("user '%s' in cache dir but not presen on system", name); + else + return pwent; + } + } + + /* Complete */ + g_dir_close (dir); + + /* Update all the users from the files in the cache dir */ + g_hash_table_iter_init (&iter, users); + while (g_hash_table_iter_next (&iter, (gpointer *)&name, (gpointer *)&user)) { + filename = g_build_filename (USERDIR, name, NULL); + key_file = g_key_file_new (); + if (g_key_file_load_from_file (key_file, filename, 0, NULL)) + user_local_update_from_keyfile (user, key_file); + g_key_file_free (key_file); + g_free (filename); + } - return g_strcmp0 (user_local_get_user_name (user), name); + *state = NULL; + return NULL; } static void -reload_passwd (Daemon *daemon) +load_entries (Daemon *daemon, + GHashTable *users, + struct passwd * (entry_generator) (GHashTable *, gpointer *)) { + gpointer generator_state = NULL; struct passwd *pwent; - GSList *old_users; - GSList *new_users; - GSList *list; -#ifdef HAVE_FGETPWENT - FILE *fp; -#endif User *user = NULL; - old_users = NULL; - new_users = NULL; + g_assert (entry_generator != NULL); -#ifdef HAVE_FGETPWENT - errno = 0; - fp = fopen (PATH_PASSWD, "r"); - if (fp == NULL) { - g_warning ("Unable to open %s: %s", PATH_PASSWD, g_strerror (errno)); - goto out; - } -#else - setpwent(); -#endif - g_hash_table_foreach (daemon->priv->users, listify_hash_values_hfunc, &old_users); - g_slist_foreach (old_users, (GFunc) g_object_ref, NULL); + for (;;) { + pwent = (entry_generator) (users, &generator_state); + if (pwent == NULL) + break; -#ifdef HAVE_FGETPWENT - while ((pwent = fgetpwent (fp)) != NULL) { -#else - while ((pwent = getpwent ()) != NULL) { -#endif /* Skip system users... */ if (daemon_local_user_is_excluded (daemon, pwent->pw_name, pwent->pw_shell)) { g_debug ("skipping user: %s", pwent->pw_name); @@ -333,9 +434,8 @@ reload_passwd (Daemon *daemon) } /* ignore duplicate entries */ - if (g_slist_find_custom (new_users, pwent->pw_name, compare_user_name)) { + if (g_hash_table_lookup (users, pwent->pw_name)) continue; - } user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name); if (user == NULL) { @@ -348,74 +448,63 @@ reload_passwd (Daemon *daemon) g_object_freeze_notify (G_OBJECT (user)); user_local_update_from_pwent (user, pwent); - new_users = g_slist_prepend (new_users, user); + g_hash_table_insert (users, g_strdup (user_local_get_user_name (user)), user); + g_debug ("loaded user: %s", user_local_get_user_name (user)); } - /* Go through and handle removed users */ - for (list = old_users; list; list = list->next) { - user = list->data; - if (! g_slist_find (new_users, user)) { - accounts_accounts_emit_user_deleted (ACCOUNTS_ACCOUNTS (daemon), user_local_get_object_path (user)); - user_local_unregister (user); - g_hash_table_remove (daemon->priv->users, - user_local_get_user_name (user)); - } - } + /* Generator should have cleaned up */ + g_assert (generator_state == NULL); +} - /* Go through and handle added users or update display names */ - for (list = new_users; list; list = list->next) { - user = list->data; - if (!g_slist_find (old_users, user)) { - user_local_register (user); - g_hash_table_insert (daemon->priv->users, - g_strdup (user_local_get_user_name (user)), - g_object_ref (user)); +static void +reload_users (Daemon *daemon) +{ + GHashTable *users; + GHashTable *old_users; + GHashTableIter iter; + gpointer name; + User *user; - accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon), user_local_get_object_path (user)); - } - } + /* Track the users that we saw during our (re)load */ + users = users_hash_table_new (); + /* Load data from all the sources, and freeze notifies */ #ifdef HAVE_FGETPWENT - out: - /* Cleanup */ - - fclose (fp); + load_entries (daemon, users, entry_generator_fgetpwent); +#else + load_entries (daemon, users, entry_generator_getpwent); #endif +#ifdef HAVE_UTMPX_H + load_entries (daemon, users, entry_generator_wtmp); +#endif + load_entries (daemon, users, entry_generator_cachedir); - g_slist_foreach (new_users, (GFunc) g_object_thaw_notify, NULL); - g_slist_foreach (new_users, (GFunc) g_object_unref, NULL); - g_slist_free (new_users); - - g_slist_foreach (old_users, (GFunc) g_object_unref, NULL); - g_slist_free (old_users); -} + /* Swap out the users */ + old_users = daemon->priv->users; + daemon->priv->users = users; -static void -reload_data (Daemon *daemon) -{ - GHashTableIter iter; - const gchar *name; - User *user; - GKeyFile *key_file; - gchar *filename; + /* Remove all the old users */ + g_hash_table_iter_init (&iter, old_users); + while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) { + if (!g_hash_table_lookup (users, name)) { + user_local_unregister (user); + accounts_accounts_emit_user_deleted (ACCOUNTS_ACCOUNTS (daemon), + user_local_get_object_path (user)); + } + } - g_hash_table_iter_init (&iter, daemon->priv->users); - while (g_hash_table_iter_next (&iter, (gpointer *)&name, (gpointer *)&user)) { - filename = g_build_filename (USERDIR, name, NULL); - key_file = g_key_file_new (); - if (g_key_file_load_from_file (key_file, filename, 0, NULL)) - user_local_update_from_keyfile (user, key_file); - g_key_file_free (key_file); - g_free (filename); + /* Register all the new users */ + g_hash_table_iter_init (&iter, users); + while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) { + if (!g_hash_table_lookup (old_users, name)) { + user_local_register (user); + accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon), + user_local_get_object_path (user)); + } + g_object_thaw_notify (G_OBJECT (user)); } -} -static void -reload_users (Daemon *daemon) -{ - reload_passwd (daemon); - reload_wtmp_history (daemon); - reload_data (daemon); + g_hash_table_destroy (old_users); } static gboolean @@ -560,10 +649,8 @@ daemon_init (Daemon *daemon) GUINT_TO_POINTER (TRUE)); } - daemon->priv->users = g_hash_table_new_full (g_str_hash, - g_str_equal, - g_free, - (GDestroyNotify) g_object_unref); + daemon->priv->users = users_hash_table_new (); + file = g_file_new_for_path (PATH_PASSWD); daemon->priv->passwd_monitor = g_file_monitor_file (file, G_FILE_MONITOR_NONE, -- 1.7.10.2