[m-dev.] diff: io__call_system/4

[Fergus Henderson]

Christopher Rodd SPEIRS, you wrote:
> Index: NEWS
>  
> +* We've changed the result produced by io__call_system/4 so that it returns 
> +  the exit code of the system call as a positive integer, or the signal that 
> +  caused the system call to exit as a negative integer.

>  %		Invokes the operating system shell with the specified
>  %		Command.  Result is either `ok(ExitStatus)', if it was
>  %		possible to invoke the command, or `error(ErrorCode)' if not.
> +%		The ExitStatus will be 0 if the command completed
> +%		successfully, the return value of the system call as a
> +%		positive integer, or the number of the signal which killed
> +%		the system call as a negative integer.

I think that is a poor interface.
We shouldn't encode values in integers like that.

I think it would be better to just return error(_)
for cases where the invoked process was killed with a signal.

> +AC_MSG_CHECKING(return values of system)
> +AC_CACHE_VAL(mercury_cv_normal_system_retval,
> +	AC_TRY_RUN([
> +	#include <stdlib.h>
> +	#ifdef HAVE_SYS_WAIT
> +	#include <sys/wait.h>
> +	#endif
> +	int main() {
> +	#if defined (WIFEXITED) && defined (WEXITSTATUS) && \
> +		defined (WIFSIGNALED) && defined (WTERMSIG)
> +	/* 
> +	** All the necessary macros for handling the return values of
> +	** system() are defined, so we do not need to test the return
> +	** value of system()
> +	*/
> +	exit(0);

Please indent that.

> +	#else
> +	/*
> +	** normal return values from system() are considered to be when
> +	** high 8 bits of the return value give the exit status, and the 
> +	** low 8 bits give the signal number which killed the process.
> +	*/
> +		if( system("exit 0") == 0 &&
> +				system("exit 42") == 42 << 8 &&
> +				system("kill -9 $$") == 9 ) {
> +			exit(0);

That won't work on Windows; gnu-win32 doesn't yet support `kill -9'.

I suggest you just comment out the call to `system("kill -9 $$") == 9'.

> +	} else {
> +		#if defined (WIFEXITED) && defined (WEXITSTATUS) && \
> +			defined (WIFSIGNALED) && defined (WTERMSIG)
> +		if (WIFEXITED(Status))
> +			Status = WEXITSTATUS(Status);
> +		else if (WIFSIGNALED(Status))
> +			Status = -WTERMSIG(Status);
> +		else
> +			fatal_error(""io__call_system_code: system() returned an unexpected value"");

It would be better to have io__call_system return error(_)
in that case, rather than calling fatal_error().

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