[m-rev.] for review: improve `mdb --window'

Simon Taylor stayl at cs.mu.OZ.AU
Wed Jan 30 18:37:22 AEDT 2002


On 30-Jan-2002, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 30-Jan-2002, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> > @@ -126,23 +126,53 @@
> >  #endif
> >  	}
> >  	if (sigemptyset(&act.sa_mask) != 0) {
> > -		perror("Mercury runtime: cannot set clear signal mask");
> > +		MR_perror("Mercury runtime: cannot set clear signal mask");
> 
> s/Mercury runtime: //
> 
> (That prefix will be added by MR_perror() -- we don't want to print it twice.)

Fixed.

> > -	if (signal(sig, handler) == SIG_ERR) {
> > -		perror(error_message);
> > +	if ((*act = signal(sig, NULL)) == SIG_ERR) {
> 
> As a style issue I think it's clearer to write that as
> 
> 	*act = signal(sig, NULL);
> 	if (*act == SIG_ERR) {
> 
> But more importantly, don't you need a cast here?  `*act' is of type
> `MR_Code *', which is not the same as the return type of signal().
> 
> It would be a good idea to test that this code compiles without any
> warnings if HAVE_SIGACTION is not defined.

It compiled without warnings.
 
> > +		MR_perror(error_message);
> 
> I think this may cause problems because the function parameter
> `error_message' may already include a "Mercury runtime: " or
> "mdb" prefix.  Please check this.

Done.

Simon.


Estimated hours taken: 10

Improve the handling of the mdb's `--window' option.
`mdb --window' now creates a window for mdb, not the program.
The main advantage of this is that redirection of the program's
input and output now works. The new behaviour should also be
implementable using the Windows API. The old behaviour is still
available with `mdb --program-in-window'.

Unfortunately, the code to open a pseudo-terminal for mdb's I/O
isn't very portable. It works on everything we run nightly tests on,
but isn't likely to work on older systems or BSD systems.

NEWS:
	Document the change.

scripts/mdb.in:
	Handle the new behaviour of `--window'.
	Add `--program-in-window'.

trace/mercury_trace_internal.c:
	If `--window' was passed in MERCURY_OPTIONS, redirect
	mdb's I/O to a window created using `xterm -S'.

	Rename occurrences of `close' to avoid gcc warnings.

runtime/mercury_trace_base.h:
runtime/mercury_trace_base.c:
	Kill the mdb window on exit.

runtime/mercury_wrapper.h:
runtime/mercury_wrapper.c:
	Add a runtime options `--mdb-in-window' for use by the mdb script.

runtime/mercury_signal.h:
runtime/mercury_signal.c:
	Add a version of MR_setup_signal which doesn't cause
	system calls to be restarted after a signal is received.
	This is needed to allow a read() from the mdb window 
	to timeout if the window failed to start.

	Add functions to get and set the action for a given signal.

configure.in:
runtime/mercury_conf.h.in:
runtime/RESERVED_MACRO_NAMES.
	Check for functions open, close, dup, dup2, fdopen, setpgid,
	fork, execlp, wait, kill, grantpt, unlockpt, pstname, tcgetattr,
	tcsetattr and ioctl.

	Check for type pid_t.

	Check for header files <fcntl.h>, <termios.h>, <sys/ioctl.h>,
	<sys/stropts.h>.

	Check for /dev/ptmx, used to open pseudo-terminals.

runtime/mercury_misc.c:
runtime/mercury_misc.h:
	Add MR_mdb_warning, which prefixes messages from the
	debugger with "mdb: ".
	
	Add MR_perror and MR_mdb_perror, which are versions
	of perror which prefix the printed message with
	"Mercury runtime: " and "mdb: " respectively.

runtime/mercury_prof_time.c:
runtime/mercury_prof.c:
runtime/mercury_memory_handlers.c:
	Don't prefix error messages passed to MR_setup_signal
	with "Mercury runtime:". MR_setup_signal now uses
	MR_perror.



diff -u runtime/mercury_signal.c runtime/mercury_signal.c
--- runtime/mercury_signal.c
+++ runtime/mercury_signal.c
@@ -126,7 +126,7 @@
 #endif
 	}
 	if (sigemptyset(&act.sa_mask) != 0) {
-		MR_perror("Mercury runtime: cannot set clear signal mask");
+		MR_perror("cannot set clear signal mask");
 		exit(1);
 	}
 	errno = 0;
@@ -152,7 +152,8 @@
 	}
 
 #else /* not HAVE_SIGACTION */
-	if ((*act = signal(sig, NULL)) == SIG_ERR) {
+	*act = signal(sig, NULL);
+	if (*act == SIG_ERR) {
 		MR_perror(error_message);
 		exit(1);
 	}
only in patch2:
--- runtime/mercury_prof_time.c	31 May 2001 06:00:15 -0000	1.2
+++ runtime/mercury_prof_time.c	30 Jan 2002 06:11:20 -0000
@@ -106,7 +106,7 @@
 	itime.it_interval.tv_usec = prof_sig_interval_in_usecs;
 
 	MR_setup_signal(MR_itimer_sig, handler, FALSE,
-		"Mercury runtime: cannot install signal handler");
+		"cannot install signal handler");
 	MR_checked_setitimer(MR_itimer_type, &itime);
 }
 
only in patch2:
--- runtime/mercury_prof.c	29 Dec 2001 07:20:11 -0000	1.19
+++ runtime/mercury_prof.c	30 Jan 2002 06:11:07 -0000
@@ -414,7 +414,7 @@
 	MR_checked_atexit(MR_prof_finish);
   #ifdef SIGINT
 	MR_setup_signal(SIGINT, prof_handle_sigint, FALSE,
-		"Mercury runtime: cannot install signal handler");
+		"cannot install signal handler");
   #endif
 #endif
 }
only in patch2:
--- runtime/mercury_memory_handlers.c	13 Mar 2001 18:02:26 -0000	1.20
+++ runtime/mercury_memory_handlers.c	30 Jan 2002 06:10:52 -0000
@@ -282,10 +282,10 @@
 #ifndef MR_MSVC_STRUCTURED_EXCEPTIONS
   #ifdef SIGBUS
 	MR_setup_signal(SIGBUS, (MR_Code *) bus_handler, TRUE,
-		"Mercury runtime: cannot set SIGBUS handler");
+		"cannot set SIGBUS handler");
   #endif
 	MR_setup_signal(SIGSEGV, (MR_Code *) segv_handler, TRUE,
-		"Mercury runtime: cannot set SIGSEGV handler");
+		"cannot set SIGSEGV handler");
 #endif
 }
 
--------------------------------------------------------------------------
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