Bug#643634: restore nested login in gdm3
Josselin Mouette
joss at debian.org
Fri Sep 30 10:20:19 UTC 2011
Le vendredi 30 septembre 2011 à 15:40 +1000, dave bl a écrit :
> Ok I got it to work :) . It was fairly trivial to make it compile TM.
> The one final issue (that I encountered) was that the configuration
> stuffwasn't setting up the xephyr command :) (I have hard-coded it in
> this diff, which I know is not ok - also needing removal is the extra
> / useless debug messages that are currently in the code).
Thanks a lot for your work. I have a few comments though.
> index a2be1e4..9d763d2 100644
> --- a/daemon/gdm-server.c
> +++ b/daemon/gdm-server.c
> @@ -733,14 +753,18 @@ gdm_server_start (GdmServer *server)
> {
> gboolean res;
> int firstserver;
> -
> + char *vtarg = NULL;
> /* HACK: force initial server to go on vt7, since 1 to 6 usually get
> * gettys after gdm startup, which interfere with X */
> firstserver = access ("/var/run/gdm/firstserver.stamp", F_OK) < 0;
> + vtarg = firstserver ? "vt7" : NULL;
> + if (!server->priv->is_nested)
> + {
> + vtarg = NULL;
> + }
>
> /* fork X server process */
> - res = gdm_server_spawn (server, firstserver ? "vt7" : NULL);
> -
> + res = gdm_server_spawn (server, vtarg);
> return res;
> }
Looks like you’re using the Ubuntu patch for defining the first VT. This
won’t apply cleanly to the Debian tree.
> @@ -1010,7 +1116,9 @@ gdm_server_init (GdmServer *server)
>
> server->priv->pid = -1;
> server->priv->command = g_strdup (X_SERVER " -br -verbose");
> + server->priv->nested_command = g_strdup ("/usr/bin/Xephyr -audit 0 -br");
> server->priv->log_dir = g_strdup (LOGDIR);
> + server->priv->is_ready = FALSE;
>
> add_ready_handler (server);
> }
I’m not really shocked to see Xephyr hardcoded. There’s not really much
point in customizing this.
> -
> +
There’s a lot of such empty changes. Please clean up your diff.
> diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c
> index da328d3..1c5e1f5 100644
> --- a/daemon/gdm-slave.c
> +++ b/daemon/gdm-slave.c
> @@ -522,27 +531,6 @@ gdm_slave_set_busy_cursor (GdmSlave *slave)
> }
>
> static void
> -gdm_slave_setup_xhost_auth (XHostAddress *host_entries, XServerInterpretedAddress *si_entries)
> -{
> - si_entries[0].type = "localuser";
> - si_entries[0].typelength = strlen ("localuser");
> - si_entries[1].type = "localuser";
> - si_entries[1].typelength = strlen ("localuser");
> -
> - si_entries[0].value = "root";
> - si_entries[0].valuelength = strlen ("root");
> - si_entries[1].value = GDM_USERNAME;
> - si_entries[1].valuelength = strlen (GDM_USERNAME);
> -
> - host_entries[0].family = FamilyServerInterpreted;
> - host_entries[0].address = (char *) &si_entries[0];
> - host_entries[0].length = sizeof (XServerInterpretedAddress);
> - host_entries[1].family = FamilyServerInterpreted;
> - host_entries[1].address = (char *) &si_entries[1];
> - host_entries[1].length = sizeof (XServerInterpretedAddress);
> -}
> -
> -static void
> gdm_slave_set_windowpath (GdmSlave *slave)
> {
> /* setting WINDOWPATH for clients */
I talked about this with upstream and removing this code is a no-no. In
order to keep this the following code needs to be refactored:
> @@ -633,6 +621,16 @@ gdm_slave_connect_to_x11_display (GdmSlave *slave)
>
> g_debug ("GdmSlave: Server is ready - opening display %s", slave->priv->display_name);
>
> + /* Nested displays are started with authorization for the parent
> + * user only. Add the GDM user. */
> + if (slave->priv->display_is_nested)
> + {
> + g_debug ("GdmSlave: Connected to display %s", slave->priv->display_name);
> + g_free (slave->priv->display_x11_authority_file);
> + gdm_slave_add_user_authorization (slave, GDM_USERNAME,
> + &slave->priv->display_x11_authority_file);
> + }
> +
> g_setenv ("DISPLAY", slave->priv->display_name, TRUE);
> g_setenv ("XAUTHORITY", slave->priv->display_x11_authority_file, TRUE);
>
> @@ -655,25 +653,9 @@ gdm_slave_connect_to_x11_display (GdmSlave *slave)
> g_warning ("Unable to connect to display %s", slave->priv->display_name);
> ret = FALSE;
> } else if (slave->priv->display_is_local) {
> - XServerInterpretedAddress si_entries[2];
> - XHostAddress host_entries[2];
> -
> g_debug ("GdmSlave: Connected to display %s", slave->priv->display_name);
> ret = TRUE;
>
> - /* Give programs run by the slave and greeter access to the
> - * display independent of current hostname
> - */
> - gdm_slave_setup_xhost_auth (host_entries, si_entries);
> -
> - gdm_error_trap_push ();
> - XAddHosts (slave->priv->server_display, host_entries,
> - G_N_ELEMENTS (host_entries));
> - XSync (slave->priv->server_display, False);
> - if (gdm_error_trap_pop ()) {
> - g_warning ("Failed to give slave programs access to the display. Trying to proceed.");
> - }
> -
> gdm_slave_set_windowpath (slave);
> } else {
> g_debug ("GdmSlave: Connected to display %s", slave->priv->display_name);
I had to remove the call to gdm_slave_setup_xhost_auth in order for the
nested functionality to work originally, because another call to this
function is added indirectly (I don’t exactly recall where). So this
requires moving the xhost code elsewhere.
> @@ -970,8 +1092,6 @@ gdm_slave_add_user_authorization (GdmSlave *slave,
> const char *username,
> char **filenamep)
> {
> - XServerInterpretedAddress si_entries[2];
> - XHostAddress host_entries[2];
> gboolean res;
> GError *error;
> char *filename;
> @@ -1009,19 +1129,6 @@ gdm_slave_add_user_authorization (GdmSlave *slave,
> }
> g_free (filename);
>
> - /* Remove access for the programs run by slave and greeter now that the
> - * user session is starting.
> - */
> - gdm_slave_setup_xhost_auth (host_entries, si_entries);
> - gdm_error_trap_push ();
> - XRemoveHosts (slave->priv->server_display, host_entries,
> - G_N_ELEMENTS (host_entries));
> - XSync (slave->priv->server_display, False);
> - if (gdm_error_trap_pop ()) {
> - g_warning ("Failed to remove slave program access to the display. Trying to proceed.");
> - }
> -
> -
> return res;
> }
Same here.
Cheers,
--
.''`. Josselin Mouette
: :' :
`. `'
`-
More information about the pkg-gnome-maintainers
mailing list