apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: testdup test fails when compiled in debug mode on Windows
Date Thu, 07 Nov 2013 16:26:08 GMT
On Thu, Nov 7, 2013 at 11:20 AM, William A. Rowe Jr. <wrowe@rowe-clan.net>wrote:

> On Thu, 7 Nov 2013 09:43:07 -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.rumph@oracle.com> 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 <mike.rumph@oracle.com> 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/

Mime
View raw message