[m-dev.] For review: coroutining changes

Fergus Henderson fjh at cs.mu.OZ.AU
Tue Dec 1 23:00:10 AEDT 1998


On 01-Dec-1998, Thomas Charles Conway <conway at cs.mu.OZ.AU> wrote:
> +++ mercury_conf.h.in	1998/11/30 04:03:26
> @@ -90,6 +90,7 @@
>  **	HAVE_GETPAGESIZE 	we have the getpagesize() system call.
>  **	HAVE_MPROTECT    	we have the mprotect() system call.
>  **	HAVE_MEMALIGN    	we have the memalign() function.
> +**	HAVE_SELECT 	   	we have the select() function.

The configure test for HAVE_SELECT actually tests for more than just the
select() function -- it also tests for various header files and macros
(e.g. FD_ZERO).
Furthermore, the code which uses that macro depends on it doing so.
The comment above should explain this.

> +static int
> +check_pending_contexts(Bool block)
> +{
> +#ifdef	HAVE_SELECT
> +
> +	int	err, max_id, n_ids;
> +	fd_set	rd_set, wr_set, ex_set;

How about using the names `read_set', `write_set', `exec_set'?
Also I think `num_ids' would be clearer than `n_ids'.

> +#else	/* !HAVE_SELECT */
> +
> +	fatal_error("select() unavailable!");

A comment here explaining why select() is needed and what it is
used for would be helpful for people porting Mercury.

> -			if (depth > 0 && tmp->owner_thread == thd
> -					|| tmp->owner_thread == NULL)
> +			if ((depth > 0 && tmp->owner_thread == thd)
> +			    || (tmp->owner_thread == (MercuryThread) NULL)) {
>  				break;

In cases where the condition doesn't fit on a single line,
the `{' should be on a line of its own.

> Index: runtime/mercury_misc.c
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/runtime/mercury_misc.c,v
> retrieving revision 1.14
> diff -u -r1.14 mercury_misc.c
> --- mercury_misc.c	1998/11/11 02:14:18	1.14
> +++ mercury_misc.c	1998/11/30 03:57:26
> @@ -523,3 +523,4 @@
>  {
>  	HASH_STRING_FUNC_BODY
>  }
> +

That was probably an unintentional change.

> Index: runtime/mercury_misc.h
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/runtime/mercury_misc.h,v
> retrieving revision 1.11
> diff -u -r1.11 mercury_misc.h
> --- mercury_misc.h	1998/11/09 10:24:38	1.11
> +++ mercury_misc.h	1998/11/30 03:57:37
> @@ -10,6 +10,7 @@
>  **			fatal_error(),
>  **			checked_malloc(),
>  **			MR_memcpy
> +**			MR_fd_zero
>  */
>  
>  #ifndef	MERCURY_MISC_H
> @@ -17,6 +18,7 @@
>  
>  #include "mercury_types.h"	/* for `Code *' */
>  #include <stdlib.h>		/* for `size_t' */
> +#include <sys/types.h>		/* for fd_set */

That #include is not portable, and so it should be inside an #ifdef.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "Binaries may die
WWW: <http://www.cs.mu.oz.au/~fjh>  |   but source code lives forever"
PGP: finger fjh at 128.250.37.3        |     -- leaked Microsoft memo.



More information about the developers mailing list