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

Fergus Henderson fjh at cs.mu.OZ.AU
Wed Jan 30 13:13:21 AEDT 2002


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.)

>  		exit(1);
>  	}
>  	errno = 0;
>  
>  	act.SIGACTION_FIELD = handler;
> -	if (sigaction(sig, &act, NULL) != 0) {
> -		perror(error_message);
> +#else /* not HAVE_SIGACTION */
> +
> +	act = handler;
> +
> +#endif /* not HAVE_SIGACTION */
> +
> +	MR_set_signal_action(sig, &act, error_message);
> +}
> +
> +void
> +MR_get_signal_action(int sig, MR_signal_action *act,
> +			const char *error_message)
> +{
> +#ifdef HAVE_SIGACTION
> +	if (sigaction(sig, NULL, act) != 0) {
> +		MR_perror(error_message);
>  		exit(1);
>  	}
>  
>  #else /* not HAVE_SIGACTION */
> -
> -	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.

> +		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.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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