[m-dev.] For review: io.m

[Fergus Henderson]

Hi Tom, thanks for that code.
I have a few comments.

> 
>  :- pred io__tmpnam(string, io__state, io__state).
>  :- mode io__tmpnam(out, di, uo) is det.
> +	% io__tmpnam(Name, IO0, IO) binds `Name' to a temporary
> +	% file name which is unique on the filesystem.
> +	% It will reside in /tmp if the TMPDIR environment variable
> +	% is not set, or in the directory specified by TMPDIR if it
> +	% is set.
>
> +:- pred io__tmpnam(string, string, string, io__state, io__state).
> +:- mode io__tmpnam(in, in, out, di, uo) is det.
> +	% io__tmpnam(Dir, Prefix, Name, IO0, IO) binds `Name' to a
> +	% temporary file name which is unique. It will reside in the
> +	% directory specified by `Dir' and have a prefix using up to the
> +	% first 5 characters of `Prefix'.

"which is unique" is a bit unclear.  I would suggest
"which is different from the name of any existing file".

> +%#include <stdio.h>
> +
> +:- pragma c_header_code("
> +#ifndef IO_HAVE_TEMPNAM
> +	#include <sys/stat.h>
> +	#include <unistd.h>
> +	extern int	mercury_io_tempnam_counter;
> +#endif
> +").
> +
> +:- pragma c_code("
> +#ifndef IO_HAVE_TEMPNAM
> +	int	mercury_io_tempnam_counter = 0;
> +#endif
> +").

s/mercury_/ML_/

The convention we decided on (which is not yet reflected in all the
source code) is that the prefix for C identifiers defined in the
mercury/library directory is `ML_'.

> +:- pragma(c_code, io__tmpnam(Dir::in, Prefix::in, FileName::out,
> +		IO0::di, IO::uo), "{
> +#ifdef	IO_HAVE_TEMPNAM
> +	String tmp;
> +
> +	tmp = (String) tempnam(Dir, Prefix);

Why cast to String here?

> +	if (tmp  == NULL) {
> +		fprintf(stderr,
> +		  ""Mercury runtime: unable to create temporary filename\\n"");
> +		exit(1);

You should just call

	fatal_error(""unable to create temporary filename"");

rather than fprintf() and exit().

> +	}
> +	incr_saved_hp_atomic((Word *) FileName,
> +		(strlen((String) tmp) + sizeof(Word)) / sizeof(Word));

The cast on FileName needs to be an `LVALUE_CAST(Word *, FileName)'.
Why cast tmp to String here?

> +	int	len, err;
> +	char	countstr[256];
> +	struct stat buf;
> +
> +	len = strlen(Dir) + 5 + 3 + 1; /* Dir + Prefix + counter + \\0 */

You forgot to count the `/'.

> +	incr_saved_hp_atomic((Word *)FileName,
> +		(len + sizeof(Word)) / sizeof(Word));

The cast on FileName needs to be an `LVALUE_CAST(Word *, FileName)'.

> +	if (mercury_io_tempnam_counter == 0)
> +		mercury_io_tempnam_counter = getpid();
> +	sprintf(countstr, ""%0d"", mercury_io_tempnam_counter % 1000);
> +	mercury_io_tempnam_counter++;
> +	strcpy(FileName, Dir);
> +	strcat(FileName, ""/"");
> +	strncat(FileName, Prefix, 5);
> +	strncat(FileName, countstr, 3);
> +	err = stat(FileName, &buf);
> +	if (err != -1 && errno != ENOENT) {
> +		fprintf(stderr,
> +		  ""Mercury runtime: unable to create temporary filename\\n"");
> +		exit(1);
> +	}

You should try this in a loop, say 5 times.
You should use fatal_error() instead of printf() and exit().

-- 
Fergus Henderson <fjh at cs.mu.oz.au>   |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>   |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3         |     -- the last words of T. S. Garp.