On Thu, Nov 7, 2013 at 11:20 AM, William A. Rowe Jr. <[email protected]>wrote:
> On Thu, 7 Nov 2013 09:43:07 -0500 > Jeff Trawick <[email protected]> wrote: > > > On Thu, Nov 7, 2013 at 12:33 AM, William A. Rowe Jr. > > <[email protected]>wrote: > > > > > On Wed, 06 Nov 2013 16:07:37 -0800 > > > Mike Rumph <[email protected]> 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 <[email protected]> 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/
