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