Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Errors-To: <wrowe@rowe-clan.net>
Message-Id: <5.1.0.14.2.20020702190436.01ef2af0@pop3.rowe-clan.net>
Date: Tue, 02 Jul 2002 19:22:41 -0500
To: Brane <brane@xbc.nu>
From: "William A. Rowe, Jr." <wrowe@rowe-clan.net>
Subject: Re: cvs commit: apr/threadproc/win32 proc.c
Cc: dev@apr.apache.org
In-Reply-To: <3D223D54.7000201@xbc.nu>
References: <5.1.0.14.2.20020702182830.032184c0@pop3.rowe-clan.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed

At 06:55 PM 7/2/2002, Brane wrote:
>William A. Rowe, Jr. wrote:
>
>>If there is a problem, it is NOT in this patch you reverted.  It is probably
>>localized to apr_file_inherit_set().  That API didn't exist when the original
>>'make inheritable duplicates' was added.

>The first order if business is to get HEAD working again, regardless.

The code wasn't correct in the first place.  Ergo reverting to a pseudo-working
state isn't productive.  Neither is reverting and then reapplying a patch.

The correct fix was 10 minutes.  A heads up to the list is ALWAYS warranted
unless vetoed code is checked into the repository.

We do NOT have to roll over each other every time that HEAD is broken.
If it doesn't compile, fine.  But you are talking about a behavioral problem
with the apr_foo_set_inherit semantics, so you rip out a perfectly legit patch.
That's bad form.

>>And you are now passing cloned parent-side handles again to the child
>>process which means the parent can't signal the file closed, because
>>closing the parent handle doesn't close the handle in the child process.
>
>I'm not sure I understand this. If you want to make the file handle 
>inheritable,
>you must create a duplicate.

Ahmm... no.  You DO NOT make the parent ends of a pipe inherited.  You
make the child ends inherited, only.  The existing code was borked.

And no, you do not need to duplicate the handle.  See the patch just committed.

I will save you the trouble of reapplying the commit you reverted.

Bill

Bill