You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/09/29 00:53:40 UTC
Re: svn commit: r580486 - in /apr/apr/trunk: include/apr_thread_proc.h
threadproc/unix/proc.c
On 09/28/2007 11:04 PM, wrowe@apache.org wrote:
> Author: wrowe
> Date: Fri Sep 28 14:04:47 2007
> New Revision: 580486
>
> URL: http://svn.apache.org/viewvc?rev=580486&view=rev
> Log:
> Introduce APR_NO_FILE as an option for any of the three stdio streams
> to cause the specified streams to be closed to the child process,
> when the caller has chosen that flag via apr_procattr_io_set().
>
> ALSO; solve a serious flaw where we attempted to dup2 to a non existant
> file if the user had not already called apr_procattr_io_set()!
>
> The Unix implementation.
>
> Modified:
> apr/apr/trunk/include/apr_thread_proc.h
> apr/apr/trunk/threadproc/unix/proc.c
>
> Modified: apr/apr/trunk/threadproc/unix/proc.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/unix/proc.c?rev=580486&r1=580485&r2=580486&view=diff
> ==============================================================================
> --- apr/apr/trunk/threadproc/unix/proc.c (original)
> +++ apr/apr/trunk/threadproc/unix/proc.c Fri Sep 28 14:04:47 2007
> @@ -551,15 +598,15 @@
> }
>
> /* Parent process */
> - if (attr->child_in) {
> + if ((attr->child_in && (attr->child_in->filedes == -1)) {
> apr_file_close(attr->child_in);
> }
>
> - if (attr->child_out) {
> + if ((attr->child_out) && (attr->child_out->filedes == -1) {
> apr_file_close(attr->child_out);
> }
>
> - if (attr->child_err) {
> + if ((attr->child_err) && (attr->child_err->filedes == -1) {
> apr_file_close(attr->child_err);
> }
Shouldn't this be filedes != -1 instead?
I see no sense in closing invalid fds while keeping valid ones open that
would have been closed before the patch.
Regards
Rüdiger
Re: svn commit: r580486 - in /apr/apr/trunk: include/apr_thread_proc.h
threadproc/unix/proc.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/29/2007 12:53 AM, Ruediger Pluem wrote:
>
> On 09/28/2007 11:04 PM, wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Fri Sep 28 14:04:47 2007
>> New Revision: 580486
>>
>> URL: http://svn.apache.org/viewvc?rev=580486&view=rev
>> Log:
>> Introduce APR_NO_FILE as an option for any of the three stdio streams
>> to cause the specified streams to be closed to the child process,
>> when the caller has chosen that flag via apr_procattr_io_set().
>>
>> ALSO; solve a serious flaw where we attempted to dup2 to a non existant
>> file if the user had not already called apr_procattr_io_set()!
>>
>> The Unix implementation.
>>
>> Modified:
>> apr/apr/trunk/include/apr_thread_proc.h
>> apr/apr/trunk/threadproc/unix/proc.c
>>
>
>> Modified: apr/apr/trunk/threadproc/unix/proc.c
>> URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/unix/proc.c?rev=580486&r1=580485&r2=580486&view=diff
>> ==============================================================================
>> --- apr/apr/trunk/threadproc/unix/proc.c (original)
>> +++ apr/apr/trunk/threadproc/unix/proc.c Fri Sep 28 14:04:47 2007
>
>> @@ -551,15 +598,15 @@
>> }
>>
>> /* Parent process */
>> - if (attr->child_in) {
>> + if ((attr->child_in && (attr->child_in->filedes == -1)) {
>> apr_file_close(attr->child_in);
>> }
>>
>> - if (attr->child_out) {
>> + if ((attr->child_out) && (attr->child_out->filedes == -1) {
>> apr_file_close(attr->child_out);
>> }
>>
>> - if (attr->child_err) {
>> + if ((attr->child_err) && (attr->child_err->filedes == -1) {
>> apr_file_close(attr->child_err);
>> }
>
>
> Shouldn't this be filedes != -1 instead?
> I see no sense in closing invalid fds while keeping valid ones open that
> would have been closed before the patch.
Ping?
Regards
Rüdiger