From dev-return-25609-apmail-apr-dev-archive=apr.apache.org@apr.apache.org Thu Nov 7 16:26:37 2013 Return-Path: X-Original-To: apmail-apr-dev-archive@www.apache.org Delivered-To: apmail-apr-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 28A4310B6B for ; Thu, 7 Nov 2013 16:26:37 +0000 (UTC) Received: (qmail 23279 invoked by uid 500); 7 Nov 2013 16:26:35 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 22966 invoked by uid 500); 7 Nov 2013 16:26:35 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Id: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 22945 invoked by uid 99); 7 Nov 2013 16:26:34 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Nov 2013 16:26:34 +0000 X-ASF-Spam-Status: No, hits=2.7 required=5.0 tests=HTML_MESSAGE,MISSING_HEADERS,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of trawick@gmail.com designates 209.85.215.43 as permitted sender) Received: from [209.85.215.43] (HELO mail-la0-f43.google.com) (209.85.215.43) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Nov 2013 16:26:28 +0000 Received: by mail-la0-f43.google.com with SMTP id ec20so647522lab.30 for ; Thu, 07 Nov 2013 08:26:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:cc :content-type; bh=+t/7wimPeMrYYaE97ZlDguXM7smQavqnxTA8Q0qbP9k=; b=GyF/mw1owL5lWQiSdNm5+S4NYRvAmnPNjwXgQUnbD429RVi6niUp1q17BkyptOA59M 6yPvZOGIp8xUU1Ic2XB4qEpIezSmbIuKxo92uoMTTIZEc2SEW886sLq25n4yFNfP1DDb Z9U9KPDMqG9v/siSOCumBFldx1vLlb/QuPhobHd/2wrBzd5aMI0WzmEKqFE9seWeRxwE bqxThKj63MRYLUJRI3FsJasiDXoVRAbEkR1ku3MAEfDub1iqZq6JsSPqw7Mvlb+fmERl wKgOxHAotCTA820PJO6RpSYmom7ilhWTm2N8v1P8cCsulmGgKIqqiMuMYEeIg67q70bg o7kg== MIME-Version: 1.0 X-Received: by 10.152.9.2 with SMTP id v2mr1816000laa.40.1383841568080; Thu, 07 Nov 2013 08:26:08 -0800 (PST) Received: by 10.114.187.71 with HTTP; Thu, 7 Nov 2013 08:26:08 -0800 (PST) In-Reply-To: <20131107102003.2fd08023@hub> References: <527A931D.9000900@oracle.com> <527AD9C9.6070708@oracle.com> <20131106233309.62349501@hub> <20131107102003.2fd08023@hub> Date: Thu, 7 Nov 2013 11:26:08 -0500 Message-ID: Subject: Re: testdup test fails when compiled in debug mode on Windows From: Jeff Trawick Cc: APR Developer List Content-Type: multipart/alternative; boundary=089e0158b8ee17773a04ea98b900 X-Virus-Checked: Checked by ClamAV on apache.org --089e0158b8ee17773a04ea98b900 Content-Type: text/plain; charset=ISO-8859-1 On Thu, Nov 7, 2013 at 11:20 AM, William A. Rowe Jr. wrote: > On Thu, 7 Nov 2013 09:43:07 -0500 > Jeff Trawick wrote: > > > On Thu, Nov 7, 2013 at 12:33 AM, William A. Rowe Jr. > > wrote: > > > > > On Wed, 06 Nov 2013 16:07:37 -0800 > > > Mike Rumph wrote: > > > > > > > > > > > On 11/6/2013 1:06 PM, Jeff Trawick wrote: > > > > > > > > > > I just played with _commit() on stdin a bit. It turns out that > > > > > _commit(0) fails if stdin is redirected (main.exe < somefile) > > > > > but works if stdin is a tty. That's the opposite of _commit(1 > > > > > or 2). But I don't see how _commit(0) makes sense anyway, so I > > > > > simply removed the call instead of reversing the corresponding > > > > > _isatty() check in your patch. > > > > > > > > > > trunk: r1539455 > > > > > 1.5.x branch: r1539461 > > > > Okay Jeff, > > > > > > > > I just tried both stdout and stdin, and got the same results that > > > > you did. Strange but true. > > > > > > IIRC the choice to _commit ahead of any handling of stdin/out/err > > > reflected the possibility that bytes were queued/stuck in the FILE > > > or the msvcrt 'fd' (not really an fd at all) before assuming > > > ownership of the file handle. It might have been an overreaction > > > to a problem that wouldn't exist in practice. But I'd prefer if > > > this were left context sensitive to _DEBUG mode builds. > > > > > > > "The *_commit* function forces the operating system to write the file > > associated with *handle* to disk" > > > > _commit(fileno_stdout or fileno_stderr) fails if it refers to the > > terminal whether or not it is a debug build. It simply isn't a valid > > call. I called out the debug build issue in CHANGES because that is > > likely the only case where anyone would encounter a problem symptom. > > (_commit() otherwise continues to return -1/EBADF and nobody notices.) > > > > The fileno_stdin issue is even more odd as it took an opposite sense > > of _isatty(fileno_stdin) to keep it from reporting an error, but I > > don't see any connection between _commit() and stdin so it seemed > > most appropriate to simply remove the call for stdin. > > > > IOW, I don't see the need to tie this to debug builds because the > > calls are invalid whether or not the RTL pops up the dialog. > > In that case I can see the benefit to dropping the call entirely. > Maybe we've lost communication here... The calls that are invalid are, essentially, _commit(console), which we've filtered out via a check to _isatty(). _commit(file) still makes sense AFAIK and still is used. > > On Thu, 07 Nov 2013 07:09:18 -0800 > Mike Rumph wrote: > > > Do you remember if the reasons for using _commit() would distinguish > > between input and output? > > No, not offhand. In any case, they were in inverted order to clean up > any lingering input ahead of dup'ing or creating an apr file_t from an > apr_os_file_t. > > Let's axe it. > > -- Born in Roswell... married an alien... http://emptyhammock.com/ --089e0158b8ee17773a04ea98b900 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On Thu, Nov 7, 2013 at 11:20 AM, William A. Rowe Jr. <w= rowe@rowe-clan.net> wrote:
On Thu, 7 Nov 2013 09:43:0= 7 -0500
Jeff Trawick <trawick@gmail.com= > wrote:

> On Thu, Nov 7, 2013 at 12:33 AM, William A. Rowe Jr.
> <wrowe@rowe-clan.net>= wrote:
>
> > On Wed, 06 Nov 2013 16:07:37 -0800
> > Mike Rumph <mike.rump= h@oracle.com> wrote:
> >
> > >
> > > On 11/6/2013 1:06 PM, Jeff Trawick wrote:
> > > >
> > > > I just played with _commit() on stdin a bit. =A0It turn= s out that
> > > > _commit(0) fails if stdin is redirected (main.exe < = somefile)
> > > > but works if stdin is a tty. =A0That's the opposite= of _commit(1
> > > > or 2). But I don't see how _commit(0) makes sense a= nyway, so I
> > > > simply removed the call instead of reversing the corres= ponding
> > > > _isatty() check in your patch.
> > > >
> > > > trunk: r1539455
> > > > 1.5.x branch: r1539461
> > > Okay Jeff,
> > >
> > > I just tried both stdout and stdin, and got the same results= that
> > > you did. Strange but true.
> >
> > IIRC the choice to _commit ahead of any handling of stdin/out/err=
> > reflected the possibility that bytes were queued/stuck in the FIL= E
> > or the msvcrt 'fd' (not really an fd at all) before assum= ing
> > ownership of the file handle. =A0It might have been an overreacti= on
> > to a problem that wouldn't exist in practice. =A0But I'd = prefer if
> > this were left context sensitive to _DEBUG mode builds.
> >
>
> "The *_commit* function forces the operating system to writ= e the file
> associated with *handle* to disk"
>
> _commit(fileno_stdout or fileno_stderr) fails if it refers to the
> terminal whether or not it is a debug build. =A0It simply isn't a = valid
> call. =A0I called out the debug build issue in CHANGES because that is=
> likely the only case where anyone would encounter a problem symptom. > (_commit() otherwise continues to return -1/EBADF and nobody notices.)=
>
> The fileno_stdin issue is even more odd as it took an opposite sense > of _isatty(fileno_stdin) to keep it from reporting an error, but I
> don't see any connection between _commit() and stdin so it seemed<= br> > most appropriate to simply remove the call for stdin.
>
> IOW, I don't see the need to tie this to debug builds because the<= br> > calls are invalid whether or not the RTL pops up the dialog.

In that case I can see the benefit to dropping the call entirely.
=

Maybe we've lost communication here...= =A0The calls that are invalid are, essentially, _commit(console), which we= 've filtered out via a check to _isatty(). =A0_commit(file) still makes= sense AFAIK and still is used.
=A0

On Thu, 07 Nov 2013 07:09:18 -0800
Mike Rumph <mike.rumph@oracle.c= om> wrote:

> Do you remember if the reasons for using _commit() would distinguish > between input and output?

No, not offhand. =A0In any case, they were in inverted order to clean= up
any lingering input ahead of dup'ing or creating an apr file_t from an<= br> apr_os_file_t.

Let's axe it.




--
Born in Rosw= ell... married an alien...
http://emptyhammock.com/
--089e0158b8ee17773a04ea98b900--