Attachment 62959 Details for Bug 50770 – Refactor reloading of users from multiple sources

[patch] Refactor reloading of users from multiple sources

0001-Refactor-reloading-of-users-from-multiple-sources.patch (text/plain), 15.25 KB, created by Stef Walter on 2012-06-13 03:41:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stef Walter

Created: 2012-06-13 03:41:46 UTC

Size: 15.25 KB

patch

obsolete

>From 68c54502cb9a29ee0e3ebf640a1595008c7b89b2 Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw@gnome.org>
>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
>

Actions: View | Splinter Review

Attachments on bug 50770: 62643 | 62959 | 62960

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.