gdm/0004-daemon-Consolidate-session-type-and-supported-sessio.patch

From d76d6ff0761d47df938f1dab0daeeecac2feb56e Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 6 Sep 2021 08:43:28 -0400
Subject: [PATCH 4/5] daemon: Consolidate session-type and
 supported-session-types list

There's currently a bug in computing the session-type to use.

The `i > 0` check means wayland will overwrite x11 in the
transient session type list.

Morever, the separation between "session-type" and
"supported-session-types" is a little redundant. Since
supported-session-types is a sorted list, the first item should
always be the same as "session-type".

This commit addresses the bug and the redundant logic, by computing
the supported session types early in the function and indexing into
it to get the session-type.

A future cleanup could probably get rid of session-type entirely.

https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/153
---
 daemon/gdm-local-display-factory.c | 193 +++++++++++++++++------------
 1 file changed, 116 insertions(+), 77 deletions(-)

diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index 141d64c6..eba38671 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -197,164 +197,226 @@ get_preferred_display_server (GdmLocalDisplayFactory *factory)
         }
 
         if (!wayland_enabled && !xorg_enabled) {
                 return g_strdup ("none");
         }
 
         gdm_settings_direct_get_string (GDM_KEY_PREFERRED_DISPLAY_SERVER, &preferred_display_server);
 
         if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
                 if (wayland_enabled)
                         return g_strdup (preferred_display_server);
                 else
                         return g_strdup ("xorg");
         }
 
         if (g_strcmp0 (preferred_display_server, "xorg") == 0) {
                 if (xorg_enabled)
                         return g_strdup (preferred_display_server);
                 else
                         return g_strdup ("wayland");
         }
 
         if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) {
                 if (xorg_enabled)
                         return g_strdup (preferred_display_server);
         }
 
         return g_strdup ("none");
 }
 
+struct GdmDisplayServerConfiguration {
+        const char *display_server;
+        const char *key;
+        const char *binary;
+        const char *session_type;
+} display_server_configuration[] = {
+#ifdef ENABLE_WAYLAND_SUPPORT
+        { "wayland", GDM_KEY_WAYLAND_ENABLE, "/usr/bin/Xwayland", "wayland" },
+#endif
+        { "xorg", GDM_KEY_XORG_ENABLE, "/usr/bin/Xorg", "x11" },
+        { NULL, NULL, NULL },
+};
+
+static gboolean
+display_server_enabled (GdmLocalDisplayFactory *factory,
+                        const char             *display_server)
+{
+        size_t i;
+
+        for (i = 0; display_server_configuration[i].display_server != NULL; i++) {
+                const char *key = display_server_configuration[i].key;
+                const char *binary = display_server_configuration[i].binary;
+                gboolean enabled = FALSE;
+
+                if (!g_str_equal (display_server_configuration[i].display_server,
+                                  display_server))
+                        continue;
+
+                if (!gdm_settings_direct_get_boolean (key, &enabled) || !enabled)
+                        return FALSE;
+
+                if (!g_file_test (binary, G_FILE_TEST_IS_EXECUTABLE))
+                        return FALSE;
+
+                return TRUE;
+        }
+
+        return FALSE;
+}
+
 static const char *
-gdm_local_display_factory_get_session_type (GdmLocalDisplayFactory *factory,
-                                            gboolean                should_fall_back)
+get_session_type_for_display_server (GdmLocalDisplayFactory *factory,
+                                     const char             *display_server)
+{
+        size_t i;
+
+        for (i = 0; display_server_configuration[i].display_server != NULL; i++) {
+                if (!g_str_equal (display_server_configuration[i].display_server,
+                                  display_server))
+                        continue;
+
+                return display_server_configuration[i].session_type;
+        }
+
+        return NULL;
+}
+
+static char **
+gdm_local_display_factory_get_session_types (GdmLocalDisplayFactory *factory,
+                                             gboolean                should_fall_back)
 {
-        const char *session_types[3] = { NULL };
-        gsize i, session_type_index = 0;
         g_autofree gchar *preferred_display_server = NULL;
+        const char *fallback_display_server = NULL;
+        gboolean wayland_preferred = FALSE;
+        gboolean xorg_preferred = FALSE;
+        g_autoptr (GPtrArray) session_types_array = NULL;
+        char **session_types;
+
+        session_types_array = g_ptr_array_new ();
 
         preferred_display_server = get_preferred_display_server (factory);
 
-        if (g_strcmp0 (preferred_display_server, "wayland") != 0 &&
-            g_strcmp0 (preferred_display_server, "xorg") != 0)
-              return NULL;
+        g_debug ("GdmLocalDisplayFactory: Getting session type (prefers %s, falling back: %s)",
+                 preferred_display_server, should_fall_back? "yes" : "no");
 
-        for (i = 0; i < G_N_ELEMENTS (session_types) - 1; i++) {
-#ifdef ENABLE_WAYLAND_SUPPORT
-            if (i > 0 ||
-                g_strcmp0 (preferred_display_server, "wayland") == 0) {
-                    gboolean wayland_enabled = FALSE;
-                    if (gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled)) {
-                            if (wayland_enabled && g_file_test ("/usr/bin/Xwayland", G_FILE_TEST_IS_EXECUTABLE)) {
-                                    session_types[i] = "wayland";
-                                    continue;
-                            }
-                    }
-            }
-#endif
+        wayland_preferred = g_str_equal (preferred_display_server, "wayland");
+        xorg_preferred = g_str_equal (preferred_display_server, "xorg");
+
+        if (wayland_preferred)
+                fallback_display_server = "xorg";
+        else if (xorg_preferred)
+                fallback_display_server = "wayland";
+        else
+                return NULL;
 
-            if (i > 0 ||
-                g_strcmp0 (preferred_display_server, "xorg") == 0) {
-                    gboolean xorg_enabled = FALSE;
-                    if (gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled)) {
-                            if (xorg_enabled && g_file_test ("/usr/bin/Xorg", G_FILE_TEST_IS_EXECUTABLE)) {
-                                    session_types[i] = "x11";
-                                    continue;
-                            }
-                    }
-            }
+        if (!should_fall_back) {
+                if (display_server_enabled (factory, preferred_display_server))
+                      g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, preferred_display_server));
         }
 
-        if (should_fall_back)
-                session_type_index++;
+        if (display_server_enabled (factory, fallback_display_server))
+                g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, fallback_display_server));
 
-        return session_types[session_type_index];
+        if (session_types_array->len == 0)
+                return NULL;
+
+        g_ptr_array_add (session_types_array, NULL);
+
+        session_types = g_strdupv ((char **) session_types_array->pdata);
+
+        return session_types;
 }
 
 static void
 on_display_disposed (GdmLocalDisplayFactory *factory,
                      GdmDisplay             *display)
 {
         g_debug ("GdmLocalDisplayFactory: Display %p disposed", display);
 }
 
 static void
 store_display (GdmLocalDisplayFactory *factory,
                GdmDisplay             *display)
 {
         GdmDisplayStore *store;
 
         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
         gdm_display_store_add (store, display);
 }
 
 /*
   Example:
   dbus-send --system --dest=org.gnome.DisplayManager \
   --type=method_call --print-reply --reply-timeout=2000 \
   /org/gnome/DisplayManager/Manager \
   org.gnome.DisplayManager.Manager.GetDisplays
 */
 gboolean
 gdm_local_display_factory_create_transient_display (GdmLocalDisplayFactory *factory,
                                                     char                  **id,
                                                     GError                **error)
 {
         gboolean         ret;
         GdmDisplay      *display = NULL;
         gboolean         is_initial = FALSE;
         const char      *session_type;
         g_autofree gchar *preferred_display_server = NULL;
 
         g_return_val_if_fail (GDM_IS_LOCAL_DISPLAY_FACTORY (factory), FALSE);
 
         ret = FALSE;
 
         g_debug ("GdmLocalDisplayFactory: Creating transient display");
 
         preferred_display_server = get_preferred_display_server (factory);
 
 #ifdef ENABLE_USER_DISPLAY_SERVER
         if (g_strcmp0 (preferred_display_server, "wayland") == 0 ||
             g_strcmp0 (preferred_display_server, "xorg") == 0) {
-                session_type = gdm_local_display_factory_get_session_type (factory, FALSE);
+                g_auto(GStrv) session_types = NULL;
 
-                if (session_type == NULL) {
+                session_types = gdm_local_display_factory_get_session_types (factory, FALSE);
+
+                if (session_types == NULL) {
                         g_set_error_literal (error,
                                              GDM_DISPLAY_ERROR,
                                              GDM_DISPLAY_ERROR_GENERAL,
                                              "Both Wayland and Xorg are unavailable");
                         return FALSE;
                 }
 
                 display = gdm_local_display_new ();
-                g_object_set (G_OBJECT (display), "session-type", session_type, NULL);
+                g_object_set (G_OBJECT (display),
+                              "session-type", session_types[0],
+                              "supported-session-types", session_types,
+                              NULL);
                 is_initial = TRUE;
         }
 #endif
         if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) {
                 if (display == NULL) {
                         guint32 num;
 
                         num = take_next_display_number (factory);
 
                         display = gdm_legacy_display_new (num);
                 }
         }
 
         if (display == NULL) {
                 g_set_error_literal (error,
                                      GDM_DISPLAY_ERROR,
                                      GDM_DISPLAY_ERROR_GENERAL,
                                      "Invalid preferred display server configured");
                 return FALSE;
         }
 
         g_object_set (display,
                       "seat-id", "seat0",
                       "allow-timed-login", FALSE,
                       "is-initial", is_initial,
                       NULL);
 
         store_display (factory, display);
 
         if (! gdm_display_manage (display)) {
@@ -549,243 +611,220 @@ lookup_prepared_display_by_seat_id (const char *id,
 
         if (status != GDM_DISPLAY_PREPARED)
                 return FALSE;
 
         return lookup_by_seat_id (id, display, user_data);
 }
 
 static int
 on_seat0_graphics_check_timeout (gpointer user_data)
 {
         GdmLocalDisplayFactory *factory = user_data;
 
         factory->seat0_graphics_check_timeout_id = 0;
 
         /* Simply try to re-add seat0. If it is there already (i.e. CanGraphical
          * turned TRUE, then we'll find it and it will not be created again).
          */
         factory->seat0_graphics_check_timed_out = TRUE;
         ensure_display_for_seat (factory, "seat0");
 
         return G_SOURCE_REMOVE;
 }
 
 static void
 ensure_display_for_seat (GdmLocalDisplayFactory *factory,
                          const char             *seat_id)
 {
         int ret;
         gboolean seat_supports_graphics;
         gboolean is_seat0;
-        const char *session_type = "wayland";
+        g_auto (GStrv) session_types = NULL;
+        const char *legacy_session_types[] = { "x11", NULL };
         GdmDisplayStore *store;
         GdmDisplay      *display = NULL;
         g_autofree char *login_session_id = NULL;
         gboolean wayland_enabled = FALSE, xorg_enabled = FALSE;
         g_autofree gchar *preferred_display_server = NULL;
-        gboolean falling_back;
+        gboolean falling_back = FALSE;
 
         gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled);
         gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled);
 
         preferred_display_server = get_preferred_display_server (factory);
 
         if (g_strcmp0 (preferred_display_server, "none") == 0) {
                g_debug ("GdmLocalDisplayFactory: Preferred display server is none, so not creating display");
                return;
         }
 
         ret = sd_seat_can_graphical (seat_id);
 
         if (ret < 0) {
                 g_critical ("Failed to query CanGraphical information for seat %s", seat_id);
                 return;
         }
 
         if (ret == 0) {
                 g_debug ("GdmLocalDisplayFactory: System doesn't currently support graphics");
                 seat_supports_graphics = FALSE;
         } else {
                 g_debug ("GdmLocalDisplayFactory: System supports graphics");
                 seat_supports_graphics = TRUE;
         }
 
         if (g_strcmp0 (seat_id, "seat0") == 0) {
                 is_seat0 = TRUE;
 
                 falling_back = factory->num_failures > 0;
-                session_type = gdm_local_display_factory_get_session_type (factory, falling_back);
+                session_types = gdm_local_display_factory_get_session_types (factory, falling_back);
 
                 g_debug ("GdmLocalDisplayFactory: New displays on seat0 will use %s%s",
-                         session_type, falling_back? " fallback" : "");
+                         session_types[0], falling_back? " fallback" : "");
         } else {
                 is_seat0 = FALSE;
 
                 g_debug ("GdmLocalDisplayFactory: New displays on seat %s will use X11 fallback", seat_id);
                 /* Force legacy X11 for all auxiliary seats */
                 seat_supports_graphics = TRUE;
-                session_type = "x11";
+                session_types = g_strdupv ((char **) legacy_session_types);
         }
 
         /* For seat0, we have a fallback logic to still try starting it after
          * SEAT0_GRAPHICS_CHECK_TIMEOUT seconds. i.e. we simply continue even if
          * CanGraphical is unset.
          * This is ugly, but it means we'll come up eventually in some
          * scenarios where no master device is present.
          * Note that we'll force an X11 fallback even though there might be
          * cases where an wayland capable device is present and simply not marked as
          * master-of-seat. In these cases, this should likely be fixed in the
          * udev rules.
          *
          * At the moment, systemd always sets CanGraphical for non-seat0 seats.
          * This is because non-seat0 seats are defined by having master-of-seat
          * set. This means we can avoid the fallback check for non-seat0 seats,
          * which simplifies the code.
          */
         if (is_seat0) {
                 if (!seat_supports_graphics) {
                         if (!factory->seat0_graphics_check_timed_out) {
                                 if (factory->seat0_graphics_check_timeout_id == 0) {
                                         g_debug ("GdmLocalDisplayFactory: seat0 doesn't yet support graphics.  Waiting %d seconds to try again.", SEAT0_GRAPHICS_CHECK_TIMEOUT);
                                         factory->seat0_graphics_check_timeout_id = g_timeout_add_seconds (SEAT0_GRAPHICS_CHECK_TIMEOUT,
                                                                                                           on_seat0_graphics_check_timeout,
                                                                                                           factory);
 
                                 } else {
                                         /* It is not yet time to force X11 fallback. */
                                         g_debug ("GdmLocalDisplayFactory: seat0 display requested when there is no graphics support before graphics check timeout.");
                                 }
 
                                 return;
                         }
 
                         g_debug ("GdmLocalDisplayFactory: Assuming we can use seat0 for X11 even though system says it doesn't support graphics!");
                         g_debug ("GdmLocalDisplayFactory: This might indicate an issue where the framebuffer device is not tagged as master-of-seat in udev.");
                         seat_supports_graphics = TRUE;
-                        session_type = "x11";
                         wayland_enabled = FALSE;
+                        g_strfreev (session_types);
+                        session_types = g_strdupv ((char **) legacy_session_types);
                 } else {
                         g_clear_handle_id (&factory->seat0_graphics_check_timeout_id, g_source_remove);
                 }
         }
 
         if (!seat_supports_graphics)
                 return;
 
-        if (session_type != NULL)
+        if (session_types != NULL)
                 g_debug ("GdmLocalDisplayFactory: %s login display for seat %s requested",
-                         session_type, seat_id);
+                         session_types[0], seat_id);
         else if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0)
                 g_debug ("GdmLocalDisplayFactory: Legacy Xorg login display for seat %s requested",
                          seat_id);
 
         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
 
         if (is_seat0)
                 display = gdm_display_store_find (store, lookup_prepared_display_by_seat_id, (gpointer) seat_id);
         else
                 display = gdm_display_store_find (store, lookup_by_seat_id, (gpointer) seat_id);
 
         /* Ensure we don't create the same display more than once */
         if (display != NULL) {
                 g_debug ("GdmLocalDisplayFactory: display already created");
                 return;
         }
 
         /* If we already have a login window, switch to it */
         if (gdm_get_login_window_session_id (seat_id, &login_session_id)) {
                 GdmDisplay *display;
 
                 display = gdm_display_store_find (store,
                                                   lookup_by_session_id,
                                                   (gpointer) login_session_id);
                 if (display != NULL &&
                     (gdm_display_get_status (display) == GDM_DISPLAY_MANAGED ||
                      gdm_display_get_status (display) == GDM_DISPLAY_WAITING_TO_FINISH)) {
                         g_object_set (G_OBJECT (display), "status", GDM_DISPLAY_MANAGED, NULL);
                         g_debug ("GdmLocalDisplayFactory: session %s found, activating.",
                                  login_session_id);
                         gdm_activate_session_by_id (factory->connection, seat_id, login_session_id);
                         return;
                 }
         }
 
         g_debug ("GdmLocalDisplayFactory: Adding display on seat %s", seat_id);
 
 #ifdef ENABLE_USER_DISPLAY_SERVER
         if (g_strcmp0 (preferred_display_server, "wayland") == 0 ||
             g_strcmp0 (preferred_display_server, "xorg") == 0) {
                 if (is_seat0) {
-                        g_autoptr (GPtrArray) supported_session_types = NULL;
-
-                        if (session_type == NULL) {
-                                g_warning ("GdmLocalDisplayFactory: Both Wayland and Xorg sessions are unavailable");
-                                return;
-                        }
-
-                        supported_session_types = g_ptr_array_new ();
-
-                        if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
-                                if (wayland_enabled)
-                                        g_ptr_array_add (supported_session_types, "wayland");
-                        } else {
-                                if (xorg_enabled)
-                                        g_ptr_array_add (supported_session_types, "x11");
-                        }
-
-                        if (!falling_back) {
-                                if (g_strcmp0 (preferred_display_server, "wayland") == 0) {
-                                        if (xorg_enabled)
-                                                g_ptr_array_add (supported_session_types, "x11");
-                                } else {
-                                        if (wayland_enabled)
-                                                g_ptr_array_add (supported_session_types, "wayland");
-                                }
-                        }
-
-                        g_ptr_array_add (supported_session_types, NULL);
-
                         display = gdm_local_display_new ();
-                        g_object_set (G_OBJECT (display), "session-type", session_type, NULL);
-                        g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types->pdata, NULL);
+                        g_object_set (G_OBJECT (display),
+                                      "session-type", session_types[0],
+                                      "supported-session-types", session_types,
+                                      NULL);
                 }
         }
 #endif
 
         if (display == NULL) {
                 guint32 num;
-                const char *supported_session_types[] = { "x11", NULL };
 
                 num = take_next_display_number (factory);
 
                 display = gdm_legacy_display_new (num);
-                g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types, NULL);
+                g_object_set (G_OBJECT (display),
+                              "session-type", legacy_session_types[0],
+                              "supported-session-types", legacy_session_types,
+                              NULL);
         }
 
         g_object_set (display, "seat-id", seat_id, NULL);
         g_object_set (display, "is-initial", is_seat0, NULL);
 
         store_display (factory, display);
 
         /* let store own the ref */
         g_object_unref (display);
 
         if (! gdm_display_manage (display)) {
                 gdm_display_unmanage (display);
         }
 
         return;
 }
 
 static void
 delete_display (GdmLocalDisplayFactory *factory,
                 const char             *seat_id) {
 
         GdmDisplayStore *store;
 
         g_debug ("GdmLocalDisplayFactory: Removing used_display_numbers on seat %s", seat_id);
 
         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
         gdm_display_store_foreach_remove (store, lookup_by_seat_id, (gpointer) seat_id);
 }
 
 static gboolean
-- 
2.34.1