[m-rev.] for review: source-linking for mdb

Mark Brown dougl at cs.mu.OZ.AU
Tue Oct 30 15:24:43 AEDT 2001


On 29-Oct-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 29-Oct-2001, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> > On 27-Oct-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > > On 26-Oct-2001, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> > > > New commands added to mdb are 'source_window', which
> > > > opens a source-linked window if there is not one already open, and
> > > > 'source_window_close' which shuts any open source window.
> > > 
> > > Those command names are long and ugly,
> > > which is not good for interactive use.
> > > Something more concise would be much better.
> > > E.g. how about `edit' and `edit -c' (or `edit --close')?
> > 
> > I avoided that name for the reason David mentioned.  If the user edits
> > the file, it will be out of sync with the executable and its line
> > numbers.
> 
> OK, how about `view'?

That seems appropriate.  I've made all the changes suggested, apart from
the split window one which I will do later.  Below is the new log message,
as well as the diff of changes relative to the last diff I posted.

Cheers,
Mark.

Estimated hours taken: 7.5
Branches: main

Implement source-linking commands for mdb, using the new client/server
feature of vim.  New commands added to mdb are 'view', which opens a
source-linked window if there is not one already open, and 'view -c'
which shuts any open source window.

This feature requires a version of vim compiled with '+clientserver'.
If such a program cannot be found, the mdb commands produce an error
message and do nothing else.

trace/mercury_trace_internal.c:
	Implement the new mdb commands.

	Add a new global variable which stores the information required
	for one source server.
	
	Every time mdb stops at an event, synchronise the source window
	if present with the source context of the event.  At interface
	events, we use the parent context if possible, since that is more
	useful information.

	When leaving mdb, close the source window if there is one open.

trace/mercury_trace_source.c:
trace/mercury_trace_source.h:
	Code for starting up and manipulating a source-server in a vim
	window.

trace/Mmakefile:
	Add the new module to the trace library.

doc/user_guide.texi:
doc/mdb_categories:
	Document the new mdb commands.

configure.in:
runtime/mercury_conf.h.in:
	Test for the functions getpid() and gethostname().  Without
	these functions mdb uses an inferior method to generate a unique
	server name.

diff -u doc/mdb_categories doc/mdb_categories
--- doc/mdb_categories
+++ doc/mdb_categories
@@ -26,8 +26,7 @@
 document_category 400 browsing
 browsing   - Commands that let users explore the state of the computation.
              The browsing commands are `vars', `print', `browse',
-             `stack', `up', `down', `level', `current', `source_window'
-             and `source_window_close'.
+             `stack', `up', `down', `level', `current' and `view'.
 
 end
 document_category 500 breakpoint
diff -u doc/user_guide.texi doc/user_guide.texi
--- doc/user_guide.texi
+++ doc/user_guide.texi
@@ -2385,9 +2385,9 @@
 In the case that the format itself is being set,
 these options are ignored.
 @sp 1
- at item source_window [-f] [-w @var{window-cmd}] [-s @var{server-cmd}] \
-[-n @var{server-name}] [-t @var{timeout}]
- at kindex source_window (mdb command)
+ at item view [-v] [-f] [-w @var{window-cmd}] [-s @var{server-cmd}] [-n @var{server-name}] [-t @var{timeout}]
+ at itemx view -c [-v] [-s @var{server-cmd}] [-n @var{server-name}]
+ at kindex view (mdb command)
 Opens a new window displaying the source code,
 at the location of the current event.
 As mdb stops at new events,
@@ -2396,9 +2396,20 @@
 compiled with the client/server option enabled.
 @sp 1
 The debugger only updates one window at a time.
-If there is already a source window open,
+If you try to open a new source window when there is already one open,
 this command aborts with an error message.
 @sp 1
+The variant with @samp{-c} (or @samp{--close})
+does not open a new window but instead
+attempts to close a currently open source window.
+The attempt may fail if, for example,
+the user has modified the source file without saving.
+ at sp 1
+The option @samp{-v} (or @samp{--verbose})
+prints the underlying system calls before running them,
+and prints any output the calls produced.
+This is useful to find out what is wrong if the server does not start.
+ at sp 1
 The option @samp{-f} (or @samp{--force})
 stops the command from aborting if there is already a window open.
 Instead it attempts to close that window first.
@@ -2418,12 +2429,6 @@
 @sp 1
 The option @samp{-t} (or @samp{--timeout}) specifies
 the maximum number of seconds to wait for the server to start.
- at sp 1
- at item source_window_close
- at kindex source_window_close (mdb command)
-Attempts to close a currently open source window.
-The attempt may fail if, for example,
-the user has modified the source file without saving.
 @end table
 @sp 1
 @node Breakpoint commands
diff -u tests/debugger/mdb_command_test.inp tests/debugger/mdb_command_test.inp
--- tests/debugger/mdb_command_test.inp
+++ tests/debugger/mdb_command_test.inp
@@ -21,8 +21,7 @@
 level                xyzzy xyzzy xyzzy xyzzy xyzzy
 current              xyzzy xyzzy xyzzy xyzzy xyzzy
 set                  xyzzy xyzzy xyzzy xyzzy xyzzy
-source_window        xyzzy xyzzy xyzzy xyzzy xyzzy
-source_window_close  xyzzy xyzzy xyzzy xyzzy xyzzy
+view                 xyzzy xyzzy xyzzy xyzzy xyzzy
 break                xyzzy xyzzy xyzzy xyzzy xyzzy
 ignore               xyzzy xyzzy xyzzy xyzzy xyzzy
 disable              xyzzy xyzzy xyzzy xyzzy xyzzy
diff -u trace/mercury_trace_internal.c trace/mercury_trace_internal.c
--- trace/mercury_trace_internal.c
+++ trace/mercury_trace_internal.c
@@ -186,9 +186,10 @@
 static	bool	MR_parse_source_locn(char *word, const char **file, int *line);
 static	const char *MR_trace_new_source_window(const char *window_cmd,
 			const char *server_cmd, const char *server_name,
-			int timeout, bool force);
-static	void	MR_trace_maybe_sync_source_window(MR_Event_Info *event_info);
-static	void	MR_trace_maybe_close_source_window(void);
+			int timeout, bool force, bool verbose);
+static	void	MR_trace_maybe_sync_source_window(MR_Event_Info *event_info,
+			bool verbose);
+static	void	MR_trace_maybe_close_source_window(bool verbose);
 static	bool	MR_trace_options_strict_print(MR_Trace_Cmd_Info *cmd,
 			char ***words, int *word_count,
 			const char *cat, const char *item);
@@ -217,10 +218,11 @@
 			MR_Word *verbose_format, MR_Word *pretty_format, 
 			char ***words, int *word_count, const char *cat, 
 			const char *item);
-static	bool	MR_trace_options_source_window(const char **window_cmd,
+static	bool	MR_trace_options_view(const char **window_cmd,
 			const char **server_cmd, const char **server_name,
-			int *timeout, bool *force, char ***words,
-			int *word_count, const char *cat, const char*item);
+			int *timeout, bool *force, bool *verbose, bool *close,
+			char ***words, int *word_count, const char *cat,
+			const char*item);
 static	void	MR_trace_usage(const char *cat, const char *item);
 static	void	MR_trace_do_noop(void);
 
@@ -288,7 +290,7 @@
 	MR_trace_internal_ensure_init();
 
 	MR_trace_event_print_internal_report(event_info);
-	MR_trace_maybe_sync_source_window(event_info);
+	MR_trace_maybe_sync_source_window(event_info, FALSE);
 
 	/*
 	** These globals can be overwritten when we call Mercury code,
@@ -1151,40 +1153,36 @@
 		{
 			MR_trace_usage("browsing", "set");
 		}
-	} else if (streq(words[0], "source_window")) {
+	} else if (streq(words[0], "view")) {
 		const char		*window_cmd = NULL;
 		const char		*server_cmd = NULL;
 		const char		*server_name = NULL;
 		int			timeout = 8;	/* seconds */
 		bool			force = FALSE;
+		bool			verbose = FALSE;
+		bool			close = FALSE;
 		const char		*msg;
 
-		if (! MR_trace_options_source_window(&window_cmd, &server_cmd,
-				&server_name, &timeout, &force, &words,
-				&word_count, "browsing", "source_window"))
+		if (! MR_trace_options_view(&window_cmd, &server_cmd,
+				&server_name, &timeout, &force, &verbose,
+				&close, &words, &word_count, "browsing",
+				"view"))
 		{
 			; /* the usage message has already been printed */
-		}
-		else if (word_count != 1) {
-			MR_trace_usage("browsing", "source_window");
+		} else if (word_count != 1) {
+			MR_trace_usage("browsing", "view");
+		} else if (close) {
+			MR_trace_maybe_close_source_window(verbose);
 		} else {
 			msg = MR_trace_new_source_window(window_cmd,
 					server_cmd, server_name, timeout,
-					force);
+					force, verbose);
 			if (msg != NULL) {
 				fflush(MR_mdb_out);
 				fprintf(MR_mdb_err, "mdb: %s.\n", msg);
 			}
 
-			MR_trace_maybe_sync_source_window(event_info);
-		}
-	} else if (streq(words[0], "source_window_close")) {
-		const char		*msg;
-
-		if (word_count != 1) {
-			MR_trace_usage("browsing", "source_window_close");
-		} else {
-			MR_trace_maybe_close_source_window();
+			MR_trace_maybe_sync_source_window(event_info, verbose);
 		}
 	} else if (streq(words[0], "break")) {
 		MR_Proc_Spec		spec;
@@ -2286,7 +2284,7 @@
 			}
 
 			if (confirmed) {
-				MR_trace_maybe_close_source_window();
+				MR_trace_maybe_close_source_window(FALSE);
 				exit(EXIT_SUCCESS);
 			}
 		} else {
@@ -2380,8 +2378,8 @@
 }
 
 /*
-** Implement the `source_window' command.  First, check if there is a
-** server attached.  If so, either stop it or abort the command, depending
+** Implement the `view' command.  First, check if there is a server
+** attached.  If so, either stop it or abort the command, depending
 ** on whether '-f' was given.  Then, if a server name was not supplied,
 ** start a new server with a unique name (which has been MR_malloc'd),
 ** otherwise attach to the server with the supplied name (and make a
@@ -2389,7 +2387,7 @@
 */
 static const char *
 MR_trace_new_source_window(const char *window_cmd, const char *server_cmd,
-		const char *server_name, int timeout, bool force)
+		const char *server_name, int timeout, bool force, bool verbose)
 {
 	const char	*msg;
 
@@ -2398,30 +2396,38 @@
 		** We are already attached to a server.
 		*/
 		if (force) {
-			MR_trace_maybe_close_source_window();
+			MR_trace_maybe_close_source_window(verbose);
 		} else {
 			return "error: server already open (use '-f' to force)";
 		}
 	}
 
+	if (server_cmd != NULL) {
+		MR_trace_source_server.server_cmd = MR_copy_string(server_cmd);
+	} else {
+		MR_trace_source_server.server_cmd = NULL;
+	}
+
 	if (server_name == NULL)
 	{
 		msg = MR_trace_source_open_server(&MR_trace_source_server,
-				window_cmd, timeout);
+				window_cmd, timeout, verbose);
 	}
 	else
 	{
 		MR_trace_source_server.server_name =
-				MR_malloc(strlen(server_name) + 1);
-		strcpy(MR_trace_source_server.server_name, server_name);
-		msg = MR_trace_source_attach(&MR_trace_source_server, timeout);
+				MR_copy_string(server_name);
+		msg = MR_trace_source_attach(&MR_trace_source_server, timeout,
+				verbose);
 		if (msg != NULL) {
 			/*
 			** Something went wrong, so we should free the
-			** server name we allocated just above.
+			** strings we allocated just above.
 			*/
 			MR_free(MR_trace_source_server.server_name);
 			MR_trace_source_server.server_name = NULL;
+			MR_free(MR_trace_source_server.server_cmd);
+			MR_trace_source_server.server_cmd = NULL;
 		}
 	}
 
@@ -2433,7 +2439,7 @@
 ** context and ask the server to point to it, otherwise do nothing.
 */
 static	void
-MR_trace_maybe_sync_source_window(MR_Event_Info *event_info)
+MR_trace_maybe_sync_source_window(MR_Event_Info *event_info, bool verbose)
 {
 	const MR_Label_Layout	*parent;
 	const char		*filename;
@@ -2467,7 +2473,7 @@
 		}
 
 		msg = MR_trace_source_sync(&MR_trace_source_server, filename,
-				lineno);
+				lineno, verbose);
 		if (msg != NULL) {
 			fflush(MR_mdb_out);
 			fprintf(MR_mdb_err, "mdb: %s.\n", msg);
@@ -2479,12 +2485,12 @@
 ** Close a source server, if there is one attached.
 */
 static	void
-MR_trace_maybe_close_source_window(void)
+MR_trace_maybe_close_source_window(bool verbose)
 {
 	const char	*msg;
 
 	if (MR_trace_source_server.server_name != NULL) {
-		msg = MR_trace_source_close(&MR_trace_source_server);
+		msg = MR_trace_source_close(&MR_trace_source_server, verbose);
 		if (msg != NULL) {
 			fflush(MR_mdb_out);
 			fprintf(MR_mdb_err, "mdb: %s.\n", msg);
@@ -2492,6 +2498,8 @@
 
 		MR_free(MR_trace_source_server.server_name);
 		MR_trace_source_server.server_name = NULL;
+		MR_free(MR_trace_source_server.server_cmd);
+		MR_trace_source_server.server_cmd = NULL;
 	}
 }
 
@@ -2947,51 +2955,93 @@
 	return TRUE;
 }
 
-static struct MR_option MR_trace_source_window_opts[] =
+static struct MR_option MR_trace_view_opts[] =
 {
+	{ "close",		MR_no_argument,		NULL,	'c' },
 	{ "window-command",	MR_required_argument,	NULL,	'w' },
 	{ "server-command",	MR_required_argument,	NULL,	's' },
 	{ "server-name",	MR_required_argument,	NULL,	'n' },
 	{ "timeout",		MR_required_argument,	NULL,	't' },
 	{ "force",		MR_no_argument,		NULL,	'f' },
+	{ "verbose",		MR_no_argument,		NULL,	'v' },
 	{ NULL,			MR_no_argument,		NULL,	0 }
 };
 
 static bool
-MR_trace_options_source_window(const char **window_cmd,
-		const char **server_cmd, const char **server_name,
-		int *timeout, bool *force, char ***words, int *word_count,
+MR_trace_options_view(const char **window_cmd, const char **server_cmd,
+		const char **server_name, int *timeout, bool *force,
+		bool *verbose, bool *close, char ***words, int *word_count,
 		const char *cat, const char *item)
 {
 	int	c;
+	bool	no_close = FALSE;
 
 	MR_optind = 0;
-	while ((c = MR_getopt_long(*word_count, *words, "w:s:n:t:f",
-			MR_trace_source_window_opts, NULL)) != EOF)
+	while ((c = MR_getopt_long(*word_count, *words, "cw:s:n:t:fv",
+			MR_trace_view_opts, NULL)) != EOF)
 	{
+		/*
+		** Option '-c' is mutually incompatible with '-f', '-t',
+		** '-s', '-n' and '-w'.
+		*/
 		switch (c) {
 
+			case 'c':
+				if (no_close) {
+					MR_trace_usage(cat, item);
+					return FALSE;
+				}
+				*close = TRUE;
+				break;
+
 			case 'w':
+				if (*close) {
+					MR_trace_usage(cat, item);
+					return FALSE;
+				}
 				*window_cmd = MR_optarg;
+				no_close = TRUE;
 				break;
 
 			case 's':
+				if (*close) {
+					MR_trace_usage(cat, item);
+					return FALSE;
+				}
 				*server_cmd = MR_optarg;
+				no_close = TRUE;
 				break;
 
 			case 'n':
+				if (*close) {
+					MR_trace_usage(cat, item);
+					return FALSE;
+				}
 				*server_name = MR_optarg;
+				no_close = TRUE;
 				break;
 
 			case 't':
-				if (sscanf(MR_optarg, "%d", timeout) != 1) {
+				if (*close ||
+					sscanf(MR_optarg, "%d", timeout) != 1)
+				{
 					MR_trace_usage(cat, item);
 					return FALSE;
 				}
+				no_close = TRUE;
 				break;
 
 			case 'f':
+				if (*close) {
+					MR_trace_usage(cat, item);
+					return FALSE;
+				}
 				*force = TRUE;
+				no_close = TRUE;
+				break;
+
+			case 'v':
+				*verbose = TRUE;
 				break;
 
 			default:
@@ -3552,8 +3602,7 @@
 	{ "browsing", "level" },
 	{ "browsing", "current" },
 	{ "browsing", "set" },
-	{ "browsing", "source_window" },
-	{ "browsing", "source_window_close" },
+	{ "browsing", "view" },
 	{ "breakpoint", "break" },
 	{ "breakpoint", "ignore" },
 	{ "breakpoint", "enable" },
diff -u trace/mercury_trace_source.c trace/mercury_trace_source.c
--- trace/mercury_trace_source.c
+++ trace/mercury_trace_source.c
@@ -17,6 +17,7 @@
 
 #include "mercury_imp.h"
 #include "mercury_trace_source.h"
+#include "mercury_trace_internal.h"
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -25,15 +26,30 @@
   #include <unistd.h>
 #endif
 
+#ifdef HAVE_SYS_TYPES_H
+  #include <sys/types.h>	/* for getpid() */
+#endif
+
 #define MR_DEFAULT_SOURCE_WINDOW_COMMAND	"xterm -e"
 #define MR_DEFAULT_SOURCE_SERVER_COMMAND	"vim"
 
 /*
-** The name used for the server will be this basename followed by ".$PID".
+** The name used for the server will start with this basename.
 */
 #define MR_SOURCE_SERVER_BASENAME		"mdb_source_server"
 
 /*
+** This defines the number of chars of the hostname to use to
+** determine a unique server name.
+*/
+#define MR_SERVER_HOSTNAME_CHARS		32
+
+/*
+** The size of the buffer used to construct system calls in.
+*/
+#define MR_SYSCALL_BUFFER_SIZE			512
+
+/*
 ** Checks whether $DISPLAY is set to something reasonable.  If so,
 ** returns NULL, otherwise returns a warning message.
 */
@@ -43,14 +59,17 @@
 ** Checks whether the server command is valid.  If so, returns NULL,
 ** otherwise returns an error message.
 */
-static const char *MR_trace_source_check_server_cmd(const char *server_cmd);
+static const char *MR_trace_source_check_server_cmd(const char *server_cmd,
+		bool verbose);
 
 /*
 ** Checks whether a server with the given name is accessible.  If so,
 ** returns NULL, otherwise returns an error message.
 */
 static const char *MR_trace_source_check_server(const char *server_cmd,
-		const char *server_name);
+		const char *server_name, bool verbose);
+
+static int MR_verbose_system_call(const char *system_call, bool verbose);
 
 static const char *
 MR_trace_source_check_display(void)
@@ -62,10 +81,8 @@
 	}
 }
 
-#define MR_SYSCALL_BUFFER_SIZE 512
-
 static const char *
-MR_trace_source_check_server_cmd(const char *server_cmd)
+MR_trace_source_check_server_cmd(const char *server_cmd, bool verbose)
 {
 	char		system_call[MR_SYSCALL_BUFFER_SIZE];
 	int		status;
@@ -75,9 +92,9 @@
 	** output contains the string '+clientserver'.
 	*/
 	sprintf(system_call,
-	    "%s --version 2>&1 | fgrep -q '+clientserver' > /dev/null 2>&1",
-		server_cmd);
-	status = system(system_call);
+			"%s --version 2>&1 | fgrep -q '+clientserver' %s",
+			server_cmd, (verbose ? "" : "> /dev/null 2>&1"));
+	status = MR_verbose_system_call(system_call, verbose);
 	if (status == 0) {
 		return NULL;
 	} else {
@@ -86,7 +103,8 @@
 }
 
 static const char *
-MR_trace_source_check_server(const char *server_cmd, const char *server_name)
+MR_trace_source_check_server(const char *server_cmd, const char *server_name,
+		bool verbose)
 {
 	char		system_call[MR_SYSCALL_BUFFER_SIZE];
 	int		status;
@@ -96,9 +114,10 @@
 	** output contains our server name.  Server names are case
 	** insensitive.
 	*/
-	sprintf(system_call, "%s --serverlist | fgrep -iq '%s'",
-			server_cmd, server_name);
-	status = system(system_call);
+	sprintf(system_call, "%s --serverlist | fgrep -iq '%s' %s",
+			server_cmd, server_name,
+			(verbose ? "" : "> /dev/null 2>&1"));
+	status = MR_verbose_system_call(system_call, verbose);
 	if (status == 0) {
 		return NULL;
 	} else {
@@ -108,7 +127,7 @@
 
 const char *
 MR_trace_source_open_server(MR_Trace_Source_Server *server,
-		const char *window_cmd, int timeout)
+		const char *window_cmd, int timeout, bool verbose)
 {
 	const char 	*real_window_cmd;
 	const char	*real_server_cmd;
@@ -116,6 +135,7 @@
 	const char	*msg;
 	char		system_call[MR_SYSCALL_BUFFER_SIZE];
 	int		status;
+	int		base_len;
 	int		i;
 
 	if (window_cmd != NULL) {
@@ -142,24 +162,51 @@
 		return msg;
 	}
 
-	msg = MR_trace_source_check_server_cmd(real_server_cmd);
+	msg = MR_trace_source_check_server_cmd(real_server_cmd, verbose);
 	if (msg != NULL) {
 		return msg;
 	}
 
 	/*
-	** We generate a unique name by appending a number to a fixed
-	** string.  The number used is determined by trying all numbers
-	** in sequence until one is found that is not being used.
-	**
-	** XXX this is quite a slow way of doing things, and there is
-	** also a race condition, but it is difficult to see a better way.
-	** We should let the server pick a unique name for itself, but
+	** If possible, we generate a unique name of the form
+	** '<basename>.<hostname>.<pid>', but if the required functions
+	** aren't available, we fall back to appending numbers to the
+	** basename in sequence until one is found that is not being used.
+	** This is quite a slow way of doing things, and there is also a
+	** race condition, but it is difficult to see a better way.  We
+	** should let the server pick a unique name for itself, but
 	** how would you communicate this name back to this process?
 	*/
+	base_len = strlen(MR_SOURCE_SERVER_BASENAME);
+
+#if defined(HAVE_GETPID) && defined(HAVE_GETHOSTNAME)
+	/*
+	** Need to leave enough room for the pid, two '.'s and the
+	** terminating zero.
+	*/
+	name = MR_malloc(base_len + MR_SERVER_HOSTNAME_CHARS + 32);
+	strcpy(name, MR_SOURCE_SERVER_BASENAME);
+
+	/*
+	** Append the '.' and hostname.
+	*/
+	name[base_len] = '.';
+	gethostname(name + base_len + 1, MR_SERVER_HOSTNAME_CHARS);
+	/* Do this just in case gethostname didn't terminate the string: */
+	name[base_len + 1 + MR_SERVER_HOSTNAME_CHARS] = '\0';
+
+	/*
+	** Find the end of the string and append the '.' and pid.
+	*/
+	i = base_len + i + strlen(name + base_len + i);
+	sprintf(name + i, ".%ld", (long) getpid());
+#else
 	i = 0;
-	/* Need to leave enough room for the integer */
-	name = MR_malloc(strlen(MR_SOURCE_SERVER_BASENAME) + 10);
+	/*
+	** Need to leave enough room for a '.', the integer and the
+	** terminating zero.
+	*/
+	name = MR_malloc(base_len + 10);
 	do {
 		i++;
 		sprintf(name, "%s.%d", MR_SOURCE_SERVER_BASENAME, i);
@@ -167,17 +214,21 @@
 		** This should fail if there is no server with this
 		** name.
 		*/
-		msg = MR_trace_source_check_server(real_server_cmd, name);
+		msg = MR_trace_source_check_server(real_server_cmd, name,
+				verbose);
 	} while (msg == NULL);
+#endif
+
 	server->server_name = name;
 
 	/*
 	** Start the server in read-only mode, to discourage the user
 	** from trying to edit the source.
 	*/
-	sprintf(system_call, "%s %s -R --servername \"%s\" &",
-			real_window_cmd, real_server_cmd, name);
-	system(system_call);
+	sprintf(system_call, "%s %s -R --servername \"%s\" %s &",
+			real_window_cmd, real_server_cmd, name,
+			(verbose ? "" : "> /dev/null 2>&1"));
+	MR_verbose_system_call(system_call, verbose);
 
 	/*
 	** We need to wait until the source server has registered itself
@@ -187,7 +238,7 @@
 	** on mdb's terminal.
 	*/
 
-	msg = MR_trace_source_attach(server, timeout);
+	msg = MR_trace_source_attach(server, timeout, verbose);
 
 	if (msg != NULL) {
 		/*
@@ -202,7 +253,8 @@
 }
 
 const char *
-MR_trace_source_attach(MR_Trace_Source_Server *server, int timeout)
+MR_trace_source_attach(MR_Trace_Source_Server *server, int timeout,
+		bool verbose)
 {
 	const char	*real_server_cmd;
 	const char	*msg;
@@ -215,7 +267,7 @@
 	}
 
 	msg = MR_trace_source_check_server(real_server_cmd,
-			server->server_name);
+			server->server_name, verbose);
 	if (msg == NULL) {
 		return NULL;
 	}
@@ -227,7 +279,7 @@
 		sleep(1);
 
 		msg = MR_trace_source_check_server(real_server_cmd,
-				server->server_name);
+				server->server_name, verbose);
 		if (msg == NULL) {
 			return NULL;
 		}
@@ -238,7 +290,7 @@
 
 const char *
 MR_trace_source_sync(MR_Trace_Source_Server *server, const char *filename,
-		int lineno)
+		int lineno, bool verbose)
 {
 	const char	*real_server_cmd;
 	const char	*msg;
@@ -252,7 +304,7 @@
 	}
 
 	msg = MR_trace_source_check_server(real_server_cmd,
-			server->server_name);
+			server->server_name, FALSE);
 	if (msg != NULL) {
 		return msg;
 	}
@@ -263,7 +315,7 @@
 	sprintf(system_call, "%s --servername \"%s\" --remote '+%d' %s",
 			real_server_cmd, server->server_name, lineno,
 			filename);
-	status = system(system_call);
+	status = MR_verbose_system_call(system_call, verbose);
 	if (status != 0) {
 		return "warning: source synchronisation failed";
 	}
@@ -273,7 +325,7 @@
 	*/
 	sprintf(system_call, "%s --servername \"%s\" --remote-send 'z.'",
 			real_server_cmd, server->server_name);
-	status = system(system_call);
+	status = MR_verbose_system_call(system_call, verbose);
 	if (status != 0) {
 		return "warning: source synchronisation failed";
 	}
@@ -282,7 +334,7 @@
 }
 
 const char *
-MR_trace_source_close(MR_Trace_Source_Server *server)
+MR_trace_source_close(MR_Trace_Source_Server *server, bool verbose)
 {
 	const char	*real_server_cmd;
 	const char	*msg;
@@ -295,7 +347,7 @@
 	}
 
 	msg = MR_trace_source_check_server(real_server_cmd,
-			server->server_name);
+			server->server_name, verbose);
 	if (msg != NULL) {
 		return msg;
 	}
@@ -309,18 +361,40 @@
 	sprintf(system_call,
 			"%s --servername \"%s\" --remote-send '\034\016:q\n'",
 			real_server_cmd, server->server_name);
+	/*
+	** Avoid echoing the command even if --verbose is set, because
+	** the control characters may interfere with the terminal.
+	*/
 	system(system_call);
 
+#if 0
+	/*
+	** XXX This doesn't work properly because the server takes some
+	** time to close.  Sometimes the server is still open when we do
+	** the test, and this leads to a false warning.
+	*/
 	/*
 	** We expect the following to fail.  If it doesn't, the server
-	** didn't close properly.
+	** hasn't closed properly.
 	*/
 	msg = MR_trace_source_check_server(real_server_cmd,
-			server->server_name);
+			server->server_name, verbose);
 	if (msg == NULL) {
 		return "warning: failed to close source server";
 	} else {
 		return NULL;
 	}
+#else
+	return NULL;
+#endif
+}
+
+int MR_verbose_system_call(const char *system_call, bool verbose)
+{
+	if (verbose) {
+		fflush(MR_mdb_out);
+		fprintf(MR_mdb_err, "+ %s\n", system_call);
+	}
+	return system(system_call);
 }
 
diff -u trace/mercury_trace_source.h trace/mercury_trace_source.h
--- trace/mercury_trace_source.h
+++ trace/mercury_trace_source.h
@@ -45,19 +45,22 @@
 ** 	timeout		Maximum time to wait for the server to start,
 ** 			in seconds.  XXX don't rely on the times being
 ** 			accurate.
+** 	verbose		If TRUE, print out system calls before executing
+** 			them and show their output.  If FALSE the output
+** 			is redirected to /dev/null.
 **
 ** If successful it returns NULL, otherwise it returns a string describing
 ** the problem.
 */
 const char *MR_trace_source_open_server(MR_Trace_Source_Server *server,
-		const char *window_cmd, int timeout);
+		const char *window_cmd, int timeout, bool verbose);
 
 /*
 ** Attach to an already running server.  If successful it returns NULL,
 ** otherwise it returns a string describing the problem.
 */
 const char *MR_trace_source_attach(MR_Trace_Source_Server *server,
-		int timeout);
+		int timeout, bool verbose);
 
 /*
 ** Synchronise the server with the current source location.  This first
@@ -67,14 +70,15 @@
 ** problems.
 */
 const char *MR_trace_source_sync(MR_Trace_Source_Server *server,
-		const char *filename, int lineno);
+		const char *filename, int lineno, bool verbose);
 
 /*
 ** Close a server if possible.  If the server appears to be still
 ** running after this, returns a warning message.  This can happen if
 ** the user has modified the code in the source window, for example.
 */
-const char *MR_trace_source_close(MR_Trace_Source_Server *server);
+const char *MR_trace_source_close(MR_Trace_Source_Server *server,
+		bool verbose);
 
 #endif /* not MERCURY_TRACE_SOURCE_H */
 
only in patch2:
--- runtime/mercury_conf.h.in	7 Aug 2001 02:16:04 -0000	1.37
+++ runtime/mercury_conf.h.in	29 Oct 2001 17:29:33 -0000
@@ -146,6 +146,8 @@
 ** The following macros are defined iff the corresponding function or
 ** system call is available:
 **
+** 	HAVE_GETPID		we have the getpid() function.
+** 	HAVE_GETHOSTNAME	we have the gethostname() function.
 **	HAVE_SNPRINTF 		we have the snprintf() function.
 **	HAVE_VSNPRINTF 		we have the vsnprintf() function.
 **	HAVE__VSNPRINTF 	we have the _vsnprintf() function.
@@ -169,6 +171,8 @@
 **				rather than a function, so you should use
 **				#if defined(fileno) || defined(HAVE_FILENO)
 */
+#undef	HAVE_GETPID
+#undef	HAVE_GETHOSTNAME
 #undef	HAVE_SNPRINTF
 #undef	HAVE_VSNPRINTF
 #undef	HAVE__VSNPRINTF
only in patch2:
--- configure.in	5 Oct 2001 01:22:55 -0000	1.279
+++ configure.in	29 Oct 2001 17:25:26 -0000
@@ -440,6 +440,7 @@
 AC_HAVE_FUNCS(sysconf getpagesize memalign mprotect sigaction setitimer)
 AC_HAVE_FUNCS(strerror memmove dup fileno fdopen fstat stat)
 AC_HAVE_FUNCS(snprintf vsnprintf _vsnprintf)
+AC_HAVE_FUNCS(gethostname getpid)
 #-----------------------------------------------------------------------------#
 AC_CHECK_HEADER(unistd.h, HAVE_UNISTD_H=1)
 if test "$HAVE_UNISTD_H" = 1; then
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list