Cleanup for login success/failure sounds. Patch provided by Mark

2005-04-19  Brian Cameron  <brian.cameron@sun.com>

        * config/gdm.conf.in, daemon/gdm.[ch], daemon/slave.c,
          gui/gdmchooser.c, giu/gdmlogin.c:  Cleanup for
          login success/failure sounds.  Patch provided by
          Mark McLoughlin.  Refer to bug #300136.
        * daemon/gdm.c: Fix FlexiServer commands so that all
          output for a single command is sent as a single call
          to gdm_connection_write or gdm_connection_printf
          rather than building commands and sending them out
          in pieces.  Also use gdm_connection_printf rather
          than building so many temporary strings.  Patch
          provided by for GDM_SUP_QUERY_LOGOUT_ACTION, but
          I reworked the patch so the same technique is used
          for all gdmflexiserver commands.  Fixes bug
          #158799.  Patch based on one provided by Raffaele
          Sandrini <rasa@gmx.ch>
        * gui/greeter/greeter_geometry.c,
          gui/greeter/greeter_item.h,
          gui/greeter/greeter_parser.c, docs/C/gdm.xml:  Now
          support ratio scaling of SVG images.  Docs weren't
          provided with the patch so I added some.  Patch
          provided by.  Fixes bug 159524.  Patch provided by
          Vincent Untz <vincent@vuntz.net>.
        * gui/gdmlogin.c: Rearrange how the widgets are
          organized to make it look a bit better now that
          the message fields have sizes defined to avoid gdm2
          resizing.  Bit more of a fix for bug #117779.
        * README.install: Spell corrections and minor updates,
          now mention that running configure with the option
          --with-post-path=/usr/openwin/bin is recommended
          on Solaris.
        * MAINTAINERS, AUTHORS, NEWS, docs/C/gdm.xml,
          daemon/gdm.h: Fixed spelling, added contributors
          to AUTHORS file for patches made in the past
          few months.
        * docs/C/gdm.xml: Fixed bug 300138 by providing
          better sound docs in the a11y section.  Fixes
          bug #300138.  Patch provided by Andrew Case.
        * gui/gdmsetup.glade, gui/gdmsetup-strings.c: Removed
          strings "sdfg" and "asdf" since they can't be
          translated.  Fixes bug #301211.
        * gui/gdmchooser.c: Removed needless comment.

3 files changed, 104 insertions, 136 deletions

diff --git a/daemon/gdm.c b/daemon/gdm.c
index fcbd22c7..3e4abcd4 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -211,7 +211,9 @@ gboolean GdmVTAllocation = TRUE;
 gboolean GdmDisallowTCP = TRUE;
 gchar *GdmSoundProgram = NULL;
 gchar *GdmSoundOnLoginReadyFile = NULL;
+gboolean GdmSoundOnLoginSuccess = FALSE;
 gchar *GdmSoundOnLoginSuccessFile = NULL;
+gboolean GdmSoundOnLoginFailure = FALSE;
 gchar *GdmSoundOnLoginFailureFile = NULL;
 gchar *GdmConsoleCannotHandle = NULL;
 
@@ -449,8 +451,12 @@ gdm_config_parse (void)
     GdmSoundProgram = ve_config_get_string (cfg, GDM_KEY_SOUND_PROGRAM);
     GdmSoundOnLoginReadyFile = ve_config_get_string (cfg,
 	                           GDM_KEY_SOUND_ON_LOGIN_READY_FILE);
+    GdmSoundOnLoginSuccess = ve_config_get_bool (cfg,
+	                           GDM_KEY_SOUND_ON_LOGIN_SUCCESS);
     GdmSoundOnLoginSuccessFile = ve_config_get_string (cfg,
 	                             GDM_KEY_SOUND_ON_LOGIN_SUCCESS_FILE);
+    GdmSoundOnLoginFailure = ve_config_get_bool (cfg,
+	                           GDM_KEY_SOUND_ON_LOGIN_FAILURE);
     GdmSoundOnLoginFailureFile = ve_config_get_string (cfg,
 	                             GDM_KEY_SOUND_ON_LOGIN_FAILURE_FILE);
 
@@ -2782,13 +2788,10 @@ gdm_handle_message (GdmConnection *conn, const char *msg, gpointer data)
 			d->socket_conn = NULL;
 
 			if (conn != NULL) {
-				char *msg = g_strdup_printf
-					("OK %s\n", d->name);
 				gdm_connection_set_close_notify (conn,
 								 NULL, NULL);
-				if ( ! gdm_connection_write (conn, msg))
+				if ( ! gdm_connection_printf (conn, "OK %s\n", d->name))
 					gdm_display_unmanage (d);
-				g_free (msg);
 			}
 			
 			gdm_debug ("Got FLEXI_OK");
@@ -3753,64 +3756,69 @@ gdm_handle_user_message (GdmConnection *conn, const char *msg, gpointer data)
 		g_free (dispname);
 		g_free (xauthfile);
 	} else if (strcmp (msg, GDM_SUP_CONSOLE_SERVERS) == 0) {
+                GString *msg;
 		GSList *li;
 		const char *sep = " ";
-		gdm_connection_write (conn, "OK");
+
+		msg = g_string_new ("OK");
 		for (li = displays; li != NULL; li = li->next) {
 			GdmDisplay *disp = li->data;
 			if ( ! disp->console)
 				continue;
-			gdm_connection_printf (conn,
-					       "%s%s,%s,",
-					       sep,
+			g_string_append_printf (msg, "%s%s,%s,", sep,
 					       ve_sure_string (disp->name),
 					       ve_sure_string (disp->login));
 			sep = ";";
 			if (disp->type == TYPE_FLEXI_XNEST) {
-				gdm_connection_write
-					(conn,
+				g_string_append (msg, 
 					 ve_sure_string (disp->xnest_disp));
 			} else {
-				gdm_connection_printf (conn, "%d",
-						       disp->vt);
+				g_string_append_printf (msg, "%d", disp->vt);
 			}
 		}
-		gdm_connection_write (conn, "\n");
+		g_string_append (msg, "\n");
+		gdm_connection_write (conn, msg->str);
+		g_string_free (msg, TRUE);
 	} else if (strcmp (msg, GDM_SUP_ALL_SERVERS) == 0) {
+		GString *msg;
 		GSList *li;
 		const char *sep = " ";
-		gdm_connection_write (conn, "OK");
+
+		msg = g_string_new ("OK");
 		for (li = displays; li != NULL; li = li->next) {
 			GdmDisplay *disp = li->data;
-			gdm_connection_printf (conn,
-					       "%s%s,%s",
-					       sep,
+			g_string_append_printf (msg, "%s%s,%s", sep,
 					       ve_sure_string (disp->name),
 					       ve_sure_string (disp->login));
 			sep = ";";
 		}
-		gdm_connection_write (conn, "\n");
+		g_string_append (msg, "\n");
+		gdm_connection_write (conn, msg->str);
+		g_string_free (msg, TRUE);
 	} else if (strcmp (msg, GDM_SUP_GREETERPIDS) == 0) {
+		GString *msg;
 		GSList *li;
 		const char *sep = " ";
-		gdm_connection_write (conn, "OK");
+
+		msg = g_string_new ("OK");
 		for (li = displays; li != NULL; li = li->next) {
 			GdmDisplay *disp = li->data;
 			if (disp->greetpid > 0) {
-				gdm_connection_printf (conn, "%s%ld",
+				g_string_append_printf (msg, "%s%ld",
 						       sep, (long)disp->greetpid);
 				sep = ";";
 			}
 		}
-		gdm_connection_write (conn, "\n");
+		g_string_append (msg, "\n");
+		gdm_connection_write (conn, msg->str);
+		g_string_free (msg, TRUE);
 	} else if (strncmp (msg, GDM_SUP_UPDATE_CONFIG " ",
 		     strlen (GDM_SUP_UPDATE_CONFIG " ")) == 0) {
 		const char *key = 
 			&msg[strlen (GDM_SUP_UPDATE_CONFIG " ")];
 
 		if ( ! update_config (key)) {
-			char *msg = g_strdup_printf ("ERROR 50 Unsupported key <%s>\n", key);
-			gdm_connection_write (conn, msg);
+			gdm_connection_printf (conn, "ERROR 50 Unsupported key <%s>\n", key);
 		} else {
 			gdm_connection_write (conn, "OK\n");
 		}
@@ -3826,16 +3834,15 @@ gdm_handle_user_message (GdmConnection *conn, const char *msg, gpointer data)
 		val = ve_config_get_string (cfg, key);
 
 		if (val == NULL) {
-			char *msg = g_strdup_printf ("ERROR 50 Unsupported key <%s>\n", key);
-			gdm_connection_write (conn, msg);
+			gdm_connection_printf (conn, "ERROR 50 Unsupported key <%s>\n", key);
 		} else {
-			char *msg = g_strdup_printf ("OK %s\n", val);
-			gdm_connection_write (conn, msg);
+			gdm_connection_printf (conn, "OK %s\n", val);
 		}
 	} else if (strcmp (msg, GDM_SUP_QUERY_LOGOUT_ACTION) == 0) {
 		const char *sep = " ";
 		GdmDisplay *disp;
 		GdmLogoutAction logout_action;
+		GString *msg;
 
 		disp = gdm_connection_get_display (conn);
 
@@ -3850,7 +3857,7 @@ gdm_handle_user_message (GdmConnection *conn, const char *msg, gpointer data)
 			return;
 		}
 
-		gdm_connection_write (conn, "OK");
+		msg = g_string_new ("OK");
 
 		logout_action = disp->logout_action;
 		if (logout_action == GDM_LOGOUT_ACTION_NONE)
@@ -3859,34 +3866,30 @@ gdm_handle_user_message (GdmConnection *conn, const char *msg, gpointer data)
 		if (GdmSystemMenu &&
 		    disp->console &&
 		    ! ve_string_empty (GdmHaltReal)) {
-			gdm_connection_printf (conn, "%s%s",
-					       sep,
-					       GDM_SUP_LOGOUT_ACTION_HALT);
+			g_string_append_printf (msg, "%s%s", sep, GDM_SUP_LOGOUT_ACTION_HALT);
 			if (logout_action == GDM_LOGOUT_ACTION_HALT)
-				gdm_connection_write (conn, "!");
+				g_string_append (msg, "!");
 			sep = ";";
 		}
 		if (GdmSystemMenu &&
 		    disp->console &&
 		    ! ve_string_empty (GdmRebootReal)) {
-			gdm_connection_printf (conn, "%s%s",
-					       sep,
-					       GDM_SUP_LOGOUT_ACTION_REBOOT);
+			g_string_append_printf (msg, "%s%s", sep, GDM_SUP_LOGOUT_ACTION_REBOOT);
 			if (logout_action == GDM_LOGOUT_ACTION_REBOOT)
-				gdm_connection_write (conn, "!");
+				g_string_append (msg, "!");
 			sep = ";";
 		}
 		if (GdmSystemMenu &&
 		    disp->console &&
 		    ! ve_string_empty (GdmSuspendReal)) {
-			gdm_connection_printf (conn, "%s%s",
-					       sep,
-					       GDM_SUP_LOGOUT_ACTION_SUSPEND);
+			g_string_append_printf (msg, "%s%s", sep, GDM_SUP_LOGOUT_ACTION_SUSPEND);
 			if (logout_action == GDM_LOGOUT_ACTION_SUSPEND)
-				gdm_connection_write (conn, "!");
+				g_string_append (msg, "!");
 			sep = ";";
 		}
-		gdm_connection_write (conn, "\n");
+		g_string_append (msg, "\n");
+		gdm_connection_write (conn, msg->str);
+		g_string_free (msg, TRUE);
 	} else if (strncmp (msg, GDM_SUP_SET_LOGOUT_ACTION " ",
 		     strlen (GDM_SUP_SET_LOGOUT_ACTION " ")) == 0) {
 		const char *action = 
diff --git a/daemon/gdm.h b/daemon/gdm.h
index 08e5bddb..bc92345d 100644
--- a/daemon/gdm.h
+++ b/daemon/gdm.h
@@ -265,8 +265,8 @@ enum {
 #define GDM_KEY_INFO_MSG_FONT "greeter/InfoMsgFont="
 
 #define GDM_KEY_SOUND_ON_LOGIN_READY "greeter/SoundOnLogin=true"
-#define GDM_KEY_SOUND_ON_LOGIN_SUCCESS "greeter/SoundOnLoginSuccess=true"
-#define GDM_KEY_SOUND_ON_LOGIN_FAILURE "greeter/SoundOnLoginFailure=true"
+#define GDM_KEY_SOUND_ON_LOGIN_SUCCESS "greeter/SoundOnLoginSuccess=false"
+#define GDM_KEY_SOUND_ON_LOGIN_FAILURE "greeter/SoundOnLoginFailure=false"
 #define GDM_KEY_SOUND_ON_LOGIN_READY_FILE "greeter/SoundOnLoginFile="
 #define GDM_KEY_SOUND_ON_LOGIN_SUCCESS_FILE "greeter/SoundOnLoginSuccessFile="
 #define GDM_KEY_SOUND_ON_LOGIN_FAILURE_FILE "greeter/SoundOnLoginFailureFile="
@@ -535,7 +535,7 @@ void		gdm_final_cleanup	(void);
 /* if user already logged in somewhere, the ack response will be
    <display>,<vt>,<display>,<vt>,... */
 #define GDM_SOP_DISP_NUM     "DISP_NUM" /* <slave pid> <display as int> */
-/* For linux only currently */
+/* For Linux only currently */
 #define GDM_SOP_VT_NUM       "VT_NUM" /* <slave pid> <vt as int> */
 #define GDM_SOP_FLEXI_ERR    "FLEXI_ERR" /* <slave pid> <error num> */
 	/* 3 = X failed */
@@ -708,7 +708,7 @@ void		gdm_final_cleanup	(void);
  *      999 = Unknown error
  */
 #define GDM_SUP_CONSOLE_SERVERS  "CONSOLE_SERVERS" /* None */
-/* CONSOLE_SERVERS: List all console servers, useful for linux mostly
+/* CONSOLE_SERVERS: List all console servers, useful for Linux mostly
  *  Doesn't list xdmcp and xnest non-console servers
  * Supported since: 2.2.4.0
  * Arguments:  None
@@ -718,7 +718,7 @@ void		gdm_final_cleanup	(void);
  *   <server> is <display>,<logged in user>,<vt or xnest display>
  *
  *   <logged in user> can be empty in case no one logged in yet,
- *   and <vt> can be -1 if it's not known or not supported (on non-linux
+ *   and <vt> can be -1 if it's not known or not supported (on non-Linux
  *   for example).  If the display is an xnest display and is a console one
  *   (that is, it is an xnest inside another console display) it is listed
  *   and instead of vt, it lists the parent display in standard form.
@@ -831,7 +831,7 @@ void		gdm_final_cleanup	(void);
  *      over SET_SAFE_LOGOUT_ACTION.
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
- *      100 = Not authenticanted
+ *      100 = Not authenticated
  *      200 = Too many messages
  *      999 = Unknown error
  */
@@ -851,7 +851,7 @@ void		gdm_final_cleanup	(void);
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
  *      7 = Unknown logout action, or not available
- *      100 = Not authenticanted
+ *      100 = Not authenticated
  *      200 = Too many messages
  *      999 = Unknown error
  */
@@ -875,7 +875,7 @@ void		gdm_final_cleanup	(void);
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
  *      7 = Unknown logout action, or not available
- *      100 = Not authenticanted
+ *      100 = Not authenticated
  *      200 = Too many messages
  *      999 = Unknown error
  */
@@ -899,7 +899,7 @@ void		gdm_final_cleanup	(void);
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
  *      8 = Virtual terminals not supported
- *      100 = Not authenticanted
+ *      100 = Not authenticated
  *      200 = Too many messages
  *      999 = Unknown error
  */
@@ -917,7 +917,7 @@ void		gdm_final_cleanup	(void);
  *      0 = Not implemented
  *      8 = Virtual terminals not supported
  *      9 = Invalid virtual terminal number
- *      100 = Not authenticanted
+ *      100 = Not authenticated
  *      200 = Too many messages
  *      999 = Unknown error
  */
diff --git a/daemon/slave.c b/daemon/slave.c
index 7fc2a6d9..71dfa852 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -185,7 +185,9 @@ extern gboolean GdmDebug;
 extern gboolean GdmDisallowTCP;
 extern gchar *GdmSoundProgram;
 extern gchar *GdmSoundOnLoginReadyFile;
+extern gboolean GdmSoundOnLoginSuccess;
 extern gchar *GdmSoundOnLoginSuccessFile;
+extern gboolean GdmSoundOnLoginFailure;
 extern gchar *GdmSoundOnLoginFailureFile;
 
 
@@ -1750,6 +1752,41 @@ restart_the_greeter (void)
 	gdm_slave_sensitize_config ();
 }
 
+static gboolean
+play_login_sound (const char *sound_file)
+{
+	pid_t pid;
+
+	if (ve_string_empty (GdmSoundProgram) ||
+	    ve_string_empty (sound_file) ||
+	    access (GdmSoundProgram, X_OK) != 0 ||
+	    access (sound_file, F_OK) != 0)
+		return FALSE;
+
+	gdm_sigchld_block_push ();
+	gdm_sigterm_block_push ();
+
+	pid = fork ();
+	if (pid == 0)
+		gdm_unset_signals ();
+
+	gdm_sigterm_block_pop ();
+	gdm_sigchld_block_pop ();
+
+	if (pid == 0) {
+		setsid ();
+		seteuid (0);
+		setegid (0);
+		execl (GdmSoundProgram,
+		       GdmSoundProgram,
+		       sound_file,
+		       NULL);
+		_exit (0);
+	}
+
+	return TRUE;
+}
+
 static void
 gdm_slave_wait_for_login (void)
 {
@@ -1935,37 +1972,12 @@ gdm_slave_wait_for_login (void)
 			gdm_debug ("gdm_slave_wait_for_login: No login/Bad login");
 			gdm_slave_greeter_ctl_no_ret (GDM_RESET, "");
 
-			/* Play sounds if specified for a failed login*/
+			/* Play sounds if specified for a failed login */
 			if (d->console &&
-			    ! G_UNLIKELY (do_configurator) &&
-			    ! ve_string_empty (GdmSoundProgram) &&
-			    ! ve_string_empty (GdmSoundOnLoginFailureFile) &&
-			    access (GdmSoundProgram, X_OK) == 0 &&
-			    access (GdmSoundOnLoginFailureFile, F_OK) == 0) {
-
-				pid_t pid;
-
-				gdm_sigchld_block_push ();
-				gdm_sigterm_block_push ();
-				pid = fork ();
-				if (pid == 0)
-					gdm_unset_signals ();
-				gdm_sigterm_block_pop ();
-				gdm_sigchld_block_pop ();
-
-				if (pid == 0) {
-					setsid ();
-					seteuid (0);
-					setegid (0);
-					execl (GdmSoundProgram,
-					       GdmSoundProgram,
-					       GdmSoundOnLoginFailureFile,
-					       NULL);
-
-					_exit (0);
-				}
-			} else if (! G_UNLIKELY (do_configurator)) {
-				gdm_error (_("Login sound requested on non-local display or the play software cannot be run or the sound does not exist."));
+			    GdmSoundOnLoginFailure &&
+			    ! play_login_sound (GdmSoundOnLoginFailureFile)) {
+				gdm_error (_("Login sound requested on non-local display or the play "
+					     "software cannot be run or the sound does not exist."));
 			}
 		}
 	}
@@ -1984,34 +1996,11 @@ gdm_slave_wait_for_login (void)
 
 	/* Play sounds if specified for a successful login */
 	if (login != NULL &&
-		d->console &&
-		! ve_string_empty (GdmSoundProgram) &&
-		! ve_string_empty (GdmSoundOnLoginSuccessFile) &&
-		access (GdmSoundProgram, X_OK) == 0 &&
-		access (GdmSoundOnLoginSuccessFile, F_OK) == 0) {
-
-		pid_t pid;
-
-		gdm_sigchld_block_push ();
-		gdm_sigterm_block_push ();
-		pid = fork ();
-		if (pid == 0)
-			gdm_unset_signals ();
-		gdm_sigterm_block_pop ();
-		gdm_sigchld_block_pop ();
-		if (pid == 0) {
-			setsid ();
-			seteuid (0);
-			setegid (0);
-			execl (GdmSoundProgram,
-			       GdmSoundProgram,
-			       GdmSoundOnLoginSuccessFile,
-			       NULL);
-
-			_exit (0);
-		}
-	} else if (! G_UNLIKELY (do_configurator)) {
-		gdm_error (_("Login sound requested on non-local display or the play software cannot be run or the sound does not exist."));
+	    d->console &&
+	    GdmSoundOnLoginSuccess &&
+	    ! play_login_sound (GdmSoundOnLoginSuccessFile)) {
+		gdm_error (_("Login sound requested on non-local display or the play software "
+			     "cannot be run or the sound does not exist."));
 	}
 
 	gdm_debug ("gdm_slave_wait_for_login: got_login for '%s'",
@@ -4920,33 +4909,9 @@ check_for_interruption (const char *msg)
 			return TRUE;
 		case GDM_INTERRUPT_LOGIN_SOUND:
 			if (d->console &&
-			    ! ve_string_empty (GdmSoundProgram) &&
-			    ! ve_string_empty (GdmSoundOnLoginReadyFile) &&
-			    access (GdmSoundProgram, X_OK) == 0 &&
-			    access (GdmSoundOnLoginReadyFile, F_OK) == 0) {
-				pid_t pid;
-
-				gdm_sigchld_block_push ();
-				gdm_sigterm_block_push ();
-				pid = fork ();
-				if (pid == 0)
-					gdm_unset_signals ();
-				gdm_sigterm_block_pop ();
-				gdm_sigchld_block_pop ();
-
-				if (pid == 0) {
-					setsid ();
-					seteuid (0);
-					setegid (0);
-					execl (GdmSoundProgram,
-					       GdmSoundProgram,
-					       GdmSoundOnLoginReadyFile,
-					       NULL);
-
-					_exit (0);
-				}
-			} else {
-				gdm_error (_("Login sound requested on non-local display or the play software cannot be run or the sound does not exist"));
+			    ! play_login_sound (GdmSoundOnLoginReadyFile)) {
+				gdm_error (_("Login sound requested on non-local display or the play software "
+					     "cannot be run or the sound does not exist"));
 			}
 			return TRUE;
 		case GDM_INTERRUPT_SELECT_USER: