Peter N. Lundblad wrote:
> On Sun, 23 Oct 2005, Philip Martin wrote:
>
>>Ah yes, you are right. Hmm, what happens if errfile is null and
>>stderr has been closed, I think the fprintf is undefined behaviour?
>
> Now, *that's* a real problem! I'll use APR files instead.
It seems to me that if svn_io_start_cmd fails to start the command, it would be
better to return an svn_error_t rather than a message on stderr.
OK, strictly speaking, a sub-process has already been created by the time the
case that you are considering occurs; but only on platforms that use "fork" to
create the process. According to the docs, on other platforms an error in
attempting to run the command will be reported as a normal APR error (in the
parent process):
> /**
> * Specify an error function to be called in the child process if APR
> * encounters an error in the child prior to running the specified program.
> * @param attr The procattr describing the child process to be created.
> * @param errfn The function to call in the child process.
> * @remark At the present time, it will only be called from apr_proc_create()
> * on platforms where fork() is used. It will never be called on other
> * platforms, on those platforms apr_proc_create() will return the error
> * in the parent process rather than invoke the callback in the now-forked
> * child process.
> */
> APR_DECLARE(apr_status_t) apr_procattr_child_errfn_set(apr_procattr_t *attr,
> apr_child_errfn_t *errfn);
This seems like a design flaw in APR: different interfaces on different
platforms. So, the most consistent thing to do would be to make the errors on
"fork" platforms end up being reported the same way as on non-"fork" platforms,
by a simple status code. This should be done in APR, and this "errfn_set"
function should be removed. However, let's see if the approach is feasible and
desirable first.
If my approach makes sense, and we implement it in Subversion rather than APR,
the only remaining difficulty is getting the error information back from the
static callback function to our "svn_io_start_cmd". This is done by
associating user data with the pool, as described here:
> /**
> * The prototype for APR child errfn functions. (See the description
> * of apr_procattr_child_errfn_set() for more information.)
> * It is passed the following parameters:
> * @param pool Pool associated with the apr_proc_t. If your child
> * error function needs user data, associate it with this
> * pool.
> * @param err APR error code describing the error
> * @param description Text description of type of processing which failed
> */
> typedef void (apr_child_errfn_t)(apr_pool_t *proc, apr_status_t err,
> const char *description);
What say you?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Oct 23 19:50:41 2005