Re: svn commit: r17041 - in trunk/subversion: libsvn_subr

lundblad@tigris.org writes:

> Author: lundblad
> Date: Wed Oct 26 14:14:12 2005
> New Revision: 17041

> --- trunk/subversion/libsvn_subr/io.c (original)
> +++ trunk/subversion/libsvn_subr/io.c Wed Oct 26 14:14:12 2005
> @@ -1733,10 +1733,14 @@
> const char *desc)
> {
> char errbuf[256];
> + apr_file_t *stderr_handle;
> +
> + apr_file_open_stderr (&stderr_handle, pool);
>
> /* What we get from APR is in native encoding. */
> - fprintf (stderr, "%s: %s", desc, apr_strerror (status, errbuf,
> - sizeof (errbuf)));
> + apr_file_printf (stderr_handle, "%s: %s",
> + desc, apr_strerror (status, errbuf,
> + sizeof (errbuf)));

The lack of error handling prompted me to look at the implementation
of apr_file_open_stderr, it's basically just a wrapper around a
hard-coded file descriptor 2. A library cannot use that as there is
no guarantee that 2 will be open, or that is stderr if it is open.
If the application has closed stderr you may be writing to a random
file.

I think svn_io_start_cmd needs to pass errfile (possibly null) via
pool userdata, and handle_child_process_error needs to retrieve it and
then write to it if it's not null. If errfile is null I think the
error message simply has to be dropped.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org