daemon: Refactor reloading of users from multiple sources

This commit reworks how the list of users is accumulated.

It introduces the idea of an "entry generator" which returns
struct passwd entries repeatedly until all entries available
are returned.  It then reworks the wtmp and /etc/passwd
reading to work in terms of entry generators.

This refactor will make it easier to introduce other sources
of users.

https://bugs.freedesktop.org/show_bug.cgi?id=50770

1 files changed, 116 insertions, 108 deletions

diff --git a/src/daemon.c b/src/daemon.c
index d1708ca..c3977bf 100644
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -104,6 +104,8 @@ struct DaemonPrivate {
         PolkitAuthority *authority;
 };
 
+typedef struct passwd * (* EntryGeneratorFunc) (GHashTable *, gpointer *);
+
 static void daemon_accounts_accounts_iface_init (AccountsAccountsIface *iface);
 
 G_DEFINE_TYPE_WITH_CODE (Daemon, daemon, ACCOUNTS_TYPE_ACCOUNTS_SKELETON, G_IMPLEMENT_INTERFACE (ACCOUNTS_TYPE_ACCOUNTS, daemon_accounts_accounts_iface_init));
@@ -202,36 +204,43 @@ daemon_local_user_is_excluded (Daemon *daemon, const gchar *username, const gcha
         return ret;
 }
 
-static void
-reload_wtmp_history (Daemon *daemon)
-{
 #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;
 
+        if (*state == NULL) {
+                /* First iteration */
 #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);
-
+        /* Every iteration */
+        login_frequency_hash = *state;
         while ((wtmp_entry = getutxent ())) {
-                if (wtmp_entry->ut_type != USER_PROCESS)
+                if (wtmp_entry->ut_type != USER_PROCESS) {
                         continue;
+                }
 
-                if (wtmp_entry->ut_user[0] == 0)
+                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;
                 }
 
@@ -251,18 +260,19 @@ reload_wtmp_history (Daemon *daemon)
                                              GUINT_TO_POINTER (frequency));
                 }
 
+                return pwent;
         }
+
+        /* Last iteration */
         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);
+                user = g_hash_table_lookup (users, key);
                 if (user == NULL) {
-                        g_debug ("unable to lookup user '%s'", username);
                         continue;
                 }
 
@@ -271,51 +281,56 @@ reload_wtmp_history (Daemon *daemon)
 
         g_hash_table_foreach (login_frequency_hash, (GHFunc) g_free, NULL);
         g_hash_table_unref (login_frequency_hash);
-#endif /* HAVE_UTMPX_H */
+        *state = NULL;
+        return NULL;
 }
+#endif /* HAVE_UTMPX_H */
 
-static void
-listify_hash_values_hfunc (gpointer key,
-                           gpointer value,
-                           gpointer user_data)
+static struct passwd *
+entry_generator_fgetpwent (GHashTable *users,
+                           gpointer   *state)
 {
-        GSList **list = user_data;
+        struct passwd *pwent;
+        FILE *fp;
 
-        *list = g_slist_prepend (*list, value);
-}
+        /* 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;
+                }
+        }
 
-static gint
-compare_user_name (gconstpointer a, gconstpointer b)
-{
-        User *user = (User *)a;
-        const gchar *name = b;
+        /* Every iteration */
+        fp = *state;
+        pwent = fgetpwent (fp);
+        if (pwent != NULL) {
+                return pwent;
+        }
 
-        return g_strcmp0 (user_local_get_user_name (user), name);
+        /* Last iteration */
+        fclose (fp);
+        *state = NULL;
+        return NULL;
 }
 
 static void
-reload_passwd (Daemon *daemon)
+load_entries (Daemon             *daemon,
+              GHashTable         *users,
+              EntryGeneratorFunc  entry_generator)
 {
+        gpointer generator_state = NULL;
         struct passwd *pwent;
-        GSList *old_users;
-        GSList *new_users;
-        GSList *list;
-        FILE *fp;
         User *user = NULL;
 
-        old_users = NULL;
-        new_users = NULL;
+        g_assert (entry_generator != NULL);
 
-        errno = 0;
-        fp = fopen (PATH_PASSWD, "r");
-        if (fp == NULL) {
-                g_warning ("Unable to open %s: %s", PATH_PASSWD, g_strerror (errno));
-                goto out;
-        }
-        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;
 
-        while ((pwent = fgetpwent (fp)) != NULL) {
                 /* Skip system users... */
                 if (daemon_local_user_is_excluded (daemon, pwent->pw_name, pwent->pw_shell)) {
                         g_debug ("skipping user: %s", pwent->pw_name);
@@ -323,7 +338,7 @@ 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;
                 }
 
@@ -338,72 +353,67 @@ 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));
-                }
-        }
-
-        /* 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));
-
-                        accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon), user_local_get_object_path (user));
-                }
-        }
-
- out:
-        /* Cleanup */
-
-        fclose (fp);
-
-        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);
+        /* Generator should have cleaned up */
+        g_assert (generator_state == NULL);
+}
 
-        g_slist_foreach (old_users, (GFunc) g_object_unref, NULL);
-        g_slist_free (old_users);
+static GHashTable *
+create_users_hash_table (void)
+{
+        return g_hash_table_new_full (g_str_hash,
+                                      g_str_equal,
+                                      g_free,
+                                      g_object_unref);
 }
 
 static void
-reload_data (Daemon *daemon)
+reload_users (Daemon *daemon)
 {
+        GHashTable *users;
+        GHashTable *old_users;
         GHashTableIter iter;
-        const gchar *name;
+        gpointer name;
         User *user;
-        GKeyFile *key_file;
-        gchar *filename;
 
-        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);
+        /* Track the users that we saw during our (re)load */
+        users = create_users_hash_table ();
+
+        /* Load data from all the sources, and freeze notifies */
+        load_entries (daemon, users, entry_generator_fgetpwent);
+#ifdef HAVE_UTMPX_H
+        load_entries (daemon, users, entry_generator_wtmp);
+#endif
+
+        /* Swap out the users */
+        old_users = daemon->priv->users;
+        daemon->priv->users = users;
+
+        /* 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));
+                }
         }
-}
 
-static void
-reload_users (Daemon *daemon)
-{
-        reload_passwd (daemon);
-        reload_wtmp_history (daemon);
-        reload_data (daemon);
+        /* 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));
+        }
+
+        g_hash_table_destroy (old_users);
 }
 
 static gboolean
@@ -548,10 +558,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 = create_users_hash_table ();
+
         file = g_file_new_for_path (PATH_PASSWD);
         daemon->priv->passwd_monitor = g_file_monitor_file (file,
                                                             G_FILE_MONITOR_NONE,