Change the interruption stuff, to remove some races and the restart of

Sat May 11 23:38:02 2002  George Lebl <jirka@5z.com>

	* daemon/gdm.h, daemon/slave.[ch],
	  daemon/verify-(pam|crypt|shadow).c, gui/gdmlogin.c,
	  gui/greeter/greeter.c:  Change the interruption stuff, to remove
	  some races and the restart of greeters is done by return code

	* gui/gdmsetup.c, gui/gdmwm.[ch], gui/gdmlogin.c,
	  gui/greeter/greeter.c:  Add a property on the setup windows so
	  that we can raise the setup windows after a restart of the greeter,
	  this doesn't really work yet well.

	* gui/gdmsetup.c: add some more error checking and change the
	  owenership/permissions correctly on installed themes

6 files changed, 140 insertions, 72 deletions

diff --git a/daemon/gdm.h b/daemon/gdm.h
index 0ed8c1bd..347dc9b2 100644
--- a/daemon/gdm.h
+++ b/daemon/gdm.h
@@ -61,6 +61,7 @@
 #define DISPLAY_CHOSEN 20	/* successful chooser session,
 				   restart display */
 #define DISPLAY_XFAILED 64	/* X failed */
+#define DISPLAY_RESTARTGREETER 127 /* Restart greeter */
 #define DISPLAY_RESTARTGDM 128	/* Restart GDM */
 
 enum {
@@ -108,7 +109,6 @@ enum {
 /* Different login interruptions */
 #define GDM_INTERRUPT_TIMED_LOGIN 'T'
 #define GDM_INTERRUPT_CONFIGURE   'C'
-#define GDM_INTERRUPT_RESTART_GREETER 'R'
 
 /* The dreaded miscellaneous category */
 #define FIELD_SIZE 256
diff --git a/daemon/slave.c b/daemon/slave.c
index efaedb29..2704f67c 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -71,6 +71,9 @@ static gboolean do_configurator = FALSE; /* if this is true, login as root
 					  * and start the configurator */
 static gboolean do_restart_greeter = FALSE; /* if this is true, whack the
 					       greeter and try again */
+static gboolean restart_greeter_now = FALSE; /* restart_greeter_when the
+						SIGCHLD hits */
+static gboolean interrupted = FALSE;
 static gchar *ParsedAutomaticLogin = NULL;
 static gchar *ParsedTimedLogin = NULL;
 
@@ -526,14 +529,6 @@ gdm_slave_run (GdmDisplay *display)
 	    do {
 		    gdm_slave_wait_for_login (); /* wait for a password */
 
-		    if (do_restart_greeter) {
-			    do_restart_greeter = FALSE;
-			    if (greet)
-				    gdm_slave_whack_greeter ();
-			    gdm_slave_greeter ();  /* Start the greeter */
-			    continue;
-		    }
-
 		    d->logged_in = TRUE;
 		    gdm_slave_send_num (GDM_SOP_LOGGED_IN, TRUE);
 
@@ -721,17 +716,30 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 
 		_exit (0);
 	} else {
-		gdm_sigchld_block_push ();
+		/* XXX: is this a race if we don't push a sigchld block?
+		 * but then we don't get a signal for restarting the greeter */
 		/* wait for the config proggie to die */
 		if (d->sesspid > 0)
-			waitpid (d->sesspid, 0, 0);
+			/* must use the pid var here since sesspid might get
+			 * zeroed between the check and here by sigchld
+			 * handler */
+			waitpid (pid, 0, 0);
 		display->sesspid = 0;
-		gdm_sigchld_block_pop ();
 	}
 }
 
+static void
+restart_the_greeter (void)
+{
+	/* no login */
+	g_free (login);
+	login = NULL;
 
-
+	/* No restart it */
+	if (greet)
+		gdm_slave_whack_greeter ();
+	gdm_slave_greeter ();
+}
 
 static void
 gdm_slave_wait_for_login (void)
@@ -739,12 +747,21 @@ gdm_slave_wait_for_login (void)
 	g_free (login);
 	login = NULL;
 
-	/* init to a sane value */
-	do_timed_login = FALSE;
-	do_configurator = FALSE;
-
 	/* Chat with greeter */
 	while (login == NULL) {
+		/* init to a sane value */
+		do_timed_login = FALSE;
+		do_configurator = FALSE;
+
+		if (do_restart_greeter) {
+			restart_the_greeter ();
+		}
+
+		do_restart_greeter = FALSE;
+
+		/* We are NOT interrupted yet */
+		interrupted = FALSE;
+
 		/* just for paranoia's sake */
 		seteuid (0);
 		setegid (0);
@@ -756,8 +773,16 @@ gdm_slave_wait_for_login (void)
 					 d->console);
 		gdm_debug ("gdm_slave_wait_for_login: end verify for '%s'",
 			   ve_sure_string (login));
+
 		/* Complex, make sure to always handle the do_configurator
-		 * and do_timed_login after any call to gdm_verify_user */
+		 * do_timed_login and do_restart_greeter after any call
+		 * to gdm_verify_user */
+
+		if (do_restart_greeter) {
+			do_restart_greeter = FALSE;
+			restart_the_greeter ();
+			continue;
+		}
 
 		if (do_configurator) {
 			struct passwd *pwent;
@@ -783,6 +808,12 @@ gdm_slave_wait_for_login (void)
 						 d->console);
 			GdmAllowRoot = oldAllowRoot;
 
+			if (do_restart_greeter) {
+				do_restart_greeter = FALSE;
+				restart_the_greeter ();
+				continue;
+			}
+
 			/* the wanker can't remember his password */
 			if (login == NULL) {
 				gdm_debug (_("gdm_slave_wait_for_login: No login/Bad login"));
@@ -830,10 +861,11 @@ gdm_slave_wait_for_login (void)
 			 * log in in the meantime */
 			gdm_slave_greeter_ctl_no_ret (GDM_DISABLE, "");
 
+			restart_greeter_now = TRUE;
+
 			run_config (d, pwent);
-			/* note that we may never get here as the configurator
-			 * may have sighupped the main gdm server and with it
-			 * wiped us */
+
+			restart_greeter_now = FALSE;
 
 			gdm_verify_cleanup (d);
 
@@ -2547,6 +2579,21 @@ gdm_slave_child_handler (int sig)
 		       (int)pid, (int)WTERMSIG (status));
 
 	if (pid == d->greetpid && greet) {
+		if (WIFEXITED (status) &&
+		    WEXITSTATUS (status) == DISPLAY_RESTARTGREETER) {
+			greet = FALSE;
+			d->greetpid = 0;
+			gdm_slave_send_num (GDM_SOP_GREETPID, 0);
+
+			if (restart_greeter_now) {
+				gdm_slave_greeter ();
+			} else {
+				interrupted = TRUE;
+				do_restart_greeter = TRUE;
+			}
+			return;
+		}
+
 		/* just for paranoia's sake */
 		seteuid (0);
 		setegid (0);
@@ -2649,12 +2696,65 @@ gdm_slave_xioerror_handler (Display *disp)
 	}
 }
 
+static void
+check_for_interruption (const char *msg)
+{
+	/* Hell yeah we were interrupted, the greeter died */
+	if (msg == NULL) {
+		interrupted = TRUE;
+		return;
+	}
+
+	if (msg[0] == BEL) {
+		/* Different interruptions come here */
+		/* Note that we don't want to actually do anything.  We want
+		 * to just set some flag and go on and schedule it after we
+		 * dump out of the login in the main login checking loop */
+		switch (msg[1]) {
+		case GDM_INTERRUPT_TIMED_LOGIN:
+			/* only allow timed login if display is local,
+			 * it is allowed for this display (it's only allowed
+			 * for the first local display) and if it's set up
+			 * correctly */
+			if ((d->console || GdmAllowRemoteAutoLogin) 
+                            && d->timed_login_ok &&
+			    ! ve_string_empty (ParsedTimedLogin) &&
+                            strcmp (ParsedTimedLogin, "root") != 0 &&
+			    GdmTimedLoginDelay > 0) {
+				do_timed_login = TRUE;
+			}
+			break;
+		case GDM_INTERRUPT_CONFIGURE:
+			if (d->console &&
+			    GdmConfigAvailable &&
+			    GdmSystemMenu &&
+			    ! ve_string_empty (GdmConfigurator)) {
+				do_configurator = TRUE;
+			}
+			break;
+		default:
+			break;
+		}
+
+		/* this was an interruption, if it wasn't
+		 * handled then the user will just get an error as if he
+		 * entered an invalid login or passward.  Seriously BEL
+		 * cannot be part of a login/password really */
+		interrupted = TRUE;
+	}
+}
+
+
 char * 
 gdm_slave_greeter_ctl (char cmd, const char *str)
 {
     gchar buf[FIELD_SIZE];
     guchar c;
 
+    /* There is no spoon^H^H^H^H^Hgreeter */
+    if ( ! greet)
+	    return NULL;
+
     if (str)
 	g_print ("%c%c%s\n", STX, cmd, str);
     else
@@ -2666,10 +2766,13 @@ gdm_slave_greeter_ctl (char cmd, const char *str)
     } while (c && c != STX);
     
     if (fgets (buf, FIELD_SIZE-1, stdin) == NULL) {
+	    interrupted = TRUE;
 	    /* things don't seem well with the greeter, it probably died */
 	    return NULL;
     }
 
+    check_for_interruption (buf);
+
     /* don't forget to flush */
     fflush (stdin);
     
@@ -2848,58 +2951,23 @@ gdm_slave_exec_script (GdmDisplay *d, const gchar *dir, const char *login,
 }
 
 gboolean
-gdm_slave_greeter_check_interruption (const char *msg)
+gdm_slave_greeter_check_interruption (void)
 {
-	if (msg != NULL &&
-	    msg[0] == BEL) {
-		/* Different interruptions come here */
-		/* Note that we don't want to actually do anything.  We want
-		 * to just set some flag and go on and schedule it after we
-		 * dump out of the login in the main login checking loop */
-		switch (msg[1]) {
-		case GDM_INTERRUPT_TIMED_LOGIN:
-			/* only allow timed login if display is local,
-			 * it is allowed for this display (it's only allowed
-			 * for the first local display) and if it's set up
-			 * correctly */
-			if ((d->console || GdmAllowRemoteAutoLogin) 
-                            && d->timed_login_ok &&
-			    ! ve_string_empty (ParsedTimedLogin) &&
-                            strcmp (ParsedTimedLogin, "root") != 0 &&
-			    GdmTimedLoginDelay > 0) {
-				do_timed_login = TRUE;
-				return TRUE;
-			}
-			break;
-		case GDM_INTERRUPT_CONFIGURE:
-			if (d->console &&
-			    GdmConfigAvailable &&
-			    GdmSystemMenu &&
-			    ! ve_string_empty (GdmConfigurator)) {
-				do_configurator = TRUE;
-			}
-			break;
-		case GDM_INTERRUPT_RESTART_GREETER:
-			do_restart_greeter = TRUE;
-			break;
-		default:
-			break;
-		}
-
-		/* Return true, this was an interruption, if it wasn't
-		 * handled then the user will just get an error as if he
-		 * entered an invalid login or passward.  Seriously BEL
-		 * cannot be part of a login/password really */
+	if (interrupted) {
+		/* no longer interrupted */
+		interrupted = FALSE;
 		return TRUE;
+	} else {
+		return FALSE;
 	}
-	return FALSE;
 }
 
 gboolean
 gdm_slave_should_complain (void)
 {
 	if (do_timed_login ||
-	    do_configurator)
+	    do_configurator ||
+	    do_restart_greeter)
 		return FALSE;
 	return TRUE;
 }
diff --git a/daemon/slave.h b/daemon/slave.h
index f66b6570..adbfcbfb 100644
--- a/daemon/slave.h
+++ b/daemon/slave.h
@@ -26,7 +26,7 @@
 void     gdm_slave_start       (GdmDisplay *d);
 void     gdm_slave_greeter_ctl_no_ret (char cmd, const char *str);
 char    *gdm_slave_greeter_ctl (char cmd, const char *str);
-gboolean gdm_slave_greeter_check_interruption (const char *msg);
+gboolean gdm_slave_greeter_check_interruption (void);
 gboolean gdm_slave_should_complain (void);
 
 void	 gdm_slave_send		(const char *str, gboolean wait_for_usr2);
diff --git a/daemon/verify-crypt.c b/daemon/verify-crypt.c
index fdae5fd4..79a563cd 100644
--- a/daemon/verify-crypt.c
+++ b/daemon/verify-crypt.c
@@ -71,7 +71,7 @@ gdm_verify_user (GdmDisplay *d,
 	    gdm_slave_greeter_ctl_no_ret (GDM_MSG, _("Please enter your username"));
 	    login = gdm_slave_greeter_ctl (GDM_LOGIN, _("Username:"));
 	    if (login == NULL ||
-		gdm_slave_greeter_check_interruption (login)) {
+		gdm_slave_greeter_check_interruption ()) {
 		    if (local)
 			    gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
 		    g_free (login);
@@ -94,7 +94,7 @@ gdm_verify_user (GdmDisplay *d,
 	    passwd = gdm_slave_greeter_ctl (GDM_NOECHO, _("Password: "));
 	    if (passwd == NULL)
 		    passwd = g_strdup ("");
-	    if (gdm_slave_greeter_check_interruption (passwd)) {
+	    if (gdm_slave_greeter_check_interruption ()) {
 		    if (local)
 			    gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
 		    g_free (login);
diff --git a/daemon/verify-pam.c b/daemon/verify-pam.c
index 93b7fb85..449da1df 100644
--- a/daemon/verify-pam.c
+++ b/daemon/verify-pam.c
@@ -79,7 +79,7 @@ gdm_verify_pam_conv (int num_msg, const struct pam_message **msg,
 	    s = gdm_slave_greeter_ctl (GDM_PROMPT, utf8);
 	    g_free (utf8);
 
-	    if (gdm_slave_greeter_check_interruption (s)) {
+	    if (gdm_slave_greeter_check_interruption ()) {
 		    g_free (s);
 		    free (reply);
 		    return PAM_CONV_ERR;
@@ -95,7 +95,7 @@ gdm_verify_pam_conv (int num_msg, const struct pam_message **msg,
 	    utf8 = g_locale_to_utf8 (msg[replies]->msg, -1, NULL, NULL, NULL);
 	    s = gdm_slave_greeter_ctl (GDM_NOECHO, utf8);
 	    g_free (utf8);
-	    if (gdm_slave_greeter_check_interruption (s)) {
+	    if (gdm_slave_greeter_check_interruption ()) {
 		    g_free (s);
 		    free (reply);
 		    return PAM_CONV_ERR;
@@ -310,7 +310,7 @@ gdm_verify_user (GdmDisplay *d,
 	    gdm_slave_greeter_ctl_no_ret (GDM_MSG, _("Please enter your username"));
 	    login = gdm_slave_greeter_ctl (GDM_LOGIN, _("Username:"));
 	    if (login == NULL ||
-		gdm_slave_greeter_check_interruption (login)) {
+		gdm_slave_greeter_check_interruption ()) {
 		    if (started_timer)
 			    gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
 		    g_free (login);
diff --git a/daemon/verify-shadow.c b/daemon/verify-shadow.c
index 79e7c41b..84abb9bb 100644
--- a/daemon/verify-shadow.c
+++ b/daemon/verify-shadow.c
@@ -70,7 +70,7 @@ gdm_verify_user (GdmDisplay *d, const char *username, const gchar *display, gboo
 	    gdm_slave_greeter_ctl_no_ret (GDM_MSG, _("Please enter your username"));
 	    login = gdm_slave_greeter_ctl (GDM_LOGIN, _("Username:"));
 	    if (login == NULL ||
-		gdm_slave_greeter_check_interruption (login)) {
+		gdm_slave_greeter_check_interruption ()) {
 		    if (local)
 			    gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
 		    g_free (login);
@@ -103,7 +103,7 @@ gdm_verify_user (GdmDisplay *d, const char *username, const gchar *display, gboo
 	    passwd = gdm_slave_greeter_ctl (GDM_NOECHO, _("Password: "));
 	    if (passwd == NULL)
 		    passwd = g_strdup ("");
-	    if (gdm_slave_greeter_check_interruption (passwd)) {
+	    if (gdm_slave_greeter_check_interruption ()) {
 		    if (local)
 			    gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
 		    g_free (login);