[m-rev.] for review: Handle exit codes of forked processes on windows safely.

Julien Fischer jfischer at opturion.com
Thu Feb 19 11:03:23 AEDT 2015


Hi Paul,

On Wed, 18 Feb 2015, Paul Bone wrote:

> Branches: version-14_01-branch, master
>
> For review by anyone
>
> ---
>
> Handle exit codes of forked processes on windows safely.
>
> MSVC doesn't provide the WIFEXITED and other macros that help a programmer
> determine if a child process exited with an error message or was killed by a
> signal.

Windows doesn't really have a notion of signals per se.  The C and C++
libraries on Windows provide "signals" as far as they are required to by
the standards for those languages, but underneath they're not the same
as they are on Unix.
(A little light reading for those interested:
https://technet.microsoft.com/en-us/library/bb496995.aspx>

> In these cases Mercury assumes that positive numbers are exit codes
> and negative numbers mean that a process was killed by a signal.
>
> When "cl" exited with errors, returning the error code -2, mmc --make
> thought that it was killed by a signal and failed to display the compilation
> errors.
>
> This patch removes the existing code that tries to detect signals when these
> macros are not available.  As far as I know platforms like windows don't
> have a concept of SysV signals, therefore signals can only occur when using
> cygwin or some other compatibility layer which will then provide the
> WIFEXITED and other macros.
>
> I considered using the windows API directly however it didn't (as far as I
> could find) provide any concept of signals, so I rejected this idea.

Search for "Structured Exception Handling".

...

> ---
> compiler/file_util.m    |   1 +
> compiler/module_cmds.m  |  10 +++-
> compiler/process_util.m |   6 +--
> library/io.m            | 126 +++++++++++++++++++++++++++---------------------
> 4 files changed, 82 insertions(+), 61 deletions(-)
>
> index 99f3af6..3124c0a 100644
> --- a/compiler/module_cmds.m
> +++ b/compiler/module_cmds.m
> @@ -588,11 +588,17 @@ invoke_system_command_maybe_filter_output(Globals, ErrorStream, Verbosity,
>         )
>     ;
>         Result = ok(signalled(Signal)),
> +        report_error_to_stream(ErrorStream, "system command received signal "
> +            ++ int_to_string(Signal) ++ ".", !IO),
> +        % Aleo report the error to standard output, because if we raise the

s/Aleo/Also/


> +        % signal this error may not ever been seen, the process stops and
> +        % the user is confused.
> +        report_error("system command received signal "
> +            ++ int_to_string(Signal) ++ ".", !IO),
> +
>         % Make sure the current process gets the signal. Some systems (e.g.
>         % Linux) ignore SIGINT during a call to system().
>         raise_signal(Signal, !IO),
> -        report_error_to_stream(ErrorStream, "system command received signal "
> -            ++ int_to_string(Signal) ++ ".", !IO),
>         CommandSucceeded = no
>     ;
>         Result = error(Error),

...

> diff --git a/library/io.m b/library/io.m
> index 7ebf6ad..acc5a9b 100644
> --- a/library/io.m
> +++ b/library/io.m
> @@ -1663,7 +1663,15 @@
>     % Interpret the child process exit status returned by
>     % system() or wait().
>     %
> -:- func io.handle_system_command_exit_status(int) = io.res(io.system_result).
> +    % Deprecated, renamed to decode_system_command_exit_code/1.
> +    %
> +:- pragma obsolete(handle_system_command_exit_status/1).
> +:- func handle_system_command_exit_status(int) = io.res(io.system_result).

That function is not part of the publicly documented interface of the io module,
just delete it rather than making it obsolete.

> +
> +    % Interpret the child process exit status returned by
> +    % system() or wait().
> +    %
> +:- func decode_system_command_exit_code(int) = io.res(io.system_result).
>
> %-----------------------------------------------------------------------------%
> %-----------------------------------------------------------------------------%
> @@ -1822,13 +1830,13 @@
> :- pred io.read_byte_val(io.input_stream::in, int::out,
>     io::di, io::uo) is det.
>
> -    % io.call_system_code(Command, Status, Message, !IO):
> +    % io.call_system_code(Command, Status, Success, Message, !IO):
>     %
>     % Invokes the operating system shell with the specified Command.
> -    % Returns Status = 127 and Message on failure. Otherwise returns
> -    % the raw exit status from the system() call.
> +    % On success Success = yes and Status is valid. on failure Success = no and

s/on/On/

That looks ok otherwise.

Cheers,
Julien.




More information about the reviews mailing list