incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: testsuite process helpers [PATCH]
Date Wed, 02 Aug 2006 17:30:00 GMT
Farid Zaripov wrote:
>  > -----Original Message-----
>  > From: Martin Sebor [mailto:sebor@roguewave.com]
>  > Sent: Friday, July 28, 2006 4:02 AM
>  > To: stdcxx-dev@incubator.apache.org
>  > Subject: Re: testsuite process helpers (was: RE: string
>  > methods thread safety)
> [...]
> 
>   Martin, I have been updated the files rw_process.h, process.cpp and 
> 0.process.cpp. The new files are attached.

Can you please post a diff instead of the entire files? It makes
it easier to see changes.

Also, the [PATCH] bit should be first on the subject line (as
opposed to last), for two reasons: a) it's easier to spot it among
all other emails in one's inbox, and b) it makes it possible to sort
one's inbox and see all pending patches.

Thanks!

> 
>   ChangeLog:
>     * rw_process.h (rw_pid_t): The type long changed to intptr_t
>     (on Windows) and pid_t (on other platforms).

I would like to hide the actual type and avoid #including system
(including the C library) headers in test suite headers whenever
possible. In this case, will _RWSTD_SIZE_T do for rw_pid_t?

>     (rw_process_error_report_mode): New variable to enable/disable
>     rw_error() outputs within defined functions.

Is this enough? Don't we want to suppress the output of rw_note()
(and others as well)? If so, I think a more robust solution might
be for driver.cpp to provide and handle a variable controlling
whether to issue or suppress a diagnostic (and which). Something
like int rw_diag_mask each of whose bits would disable the
corresponding diagnostic.

>     (rw_wait_pid): Added the timeout parameter.

Excellent! Although I think the UNIX branch should probably try
to be more robust about handling an existing alarm (or avoiding
the function and using some other mechanism).

>     (rw_process_kill): New function to kill the specified process.

FYI: sending a process the KILL signal is considered quite drastic
since the signal cannot be caught and so the process cannot clean
up after itself and release system resources. That's why the
default behavior of the POSIX kill utility (i.e., when the -s
option is not specified) is to send the TERM signal. I think
rw_process_kill should behave similarly.

I would suggest to add a second argument like this

     rw_process_kill (rw_pid_t, int signo = -1);

When (signo == -1) the function should provide the same behavior
as the analogous code in wait_for_child() here (i.e., start by
sending SIGHUP, then SIGINT, then SIGTERM, and only as the last
resort, SIGKILL):
http://svn.apache.org/repos/asf/incubator/stdcxx/trunk/util/exec.cpp
(The Windows branch of the code can ignore the argument unless
there is some similar functionality in Win32).

>     * process.cpp: Ditto.
>     * 0.process.cpp: New test exercising the rw_process_create(),
>     rw_process_kill() and rw_waitpid() functions.

The test doesn't compile with strict compilers (such as EDG eccp).
Keep in mind that extern "C" and extern "C++" functions or pointers
to such things are incompatible. I.e., this is ill-formed:

   extern "C" void handle_signal (int) { }
   void foo () {
       void (*old_handler)(int) = signal (SIGALRM, handle_signal);
   }

because old_handler has extern "C++" language linkage (there's
no way to declare a local extern "C" pointer in an extern "C++"
function without introducing a typedef at namespace scope, like
so:

   extern "C" {
       typedef void sighandler_t (int);
       void handle_signal (int) { }
   }
   void foo () {
       sighandler_t* old_handler = signal (SIGALRM, handle_signal);
   }

Thanks
Martin

Mime
View raw message