On Thu, Jun 11, 2009 at 8:22 AM, Eric MALENFANT < Eric.Malenfant@sagem-interstar.com> wrote:
Zachary Turner wrote:
nested file_descriptor implementation class is defined as follows: [snip] In the second constructor in particular we have this:
impl(handle_type fd, bool close_on_exit) : handle_(fd), flags_(0) { if (close_on_exit) flags_ |= impl::close_on_exit; }
I think this should be changed. Just because close_on_exit is specified to false does not mean a file is not open. It just means don't close it if it is. I think flags enumeration should be changed to:
enum flags { close_on_exit = 1, good = 2, append = 4 };
and constructors / code should be changed accordingly. Otherwise following code (Windows) asserts, even though I don't think it should:
bool is_open(HANDLE h) { return h && h!=INVALID_HANDLE_VALUE;}
HANDLE h = ::CreateFile("C:\\autoexec.bat", GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE,NULL,OPEN_EXISTING,FILE_ATTRI BUTE_NORMAL,NULL);
boost::iostreams::file_descriptor fd(h, false);
BOOST_ASSERT(fd.is_open() == is_open(h));
Thoughts?
Wouldn't changing: bool is_open() const { return pimpl_->flags_ != 0; } to: bool is_open() const { return pimpl_->handle_ != -1; } be a better and/or simpler fix?
I blame the fact that it was 1 AM when I wrote that. Not really sure what I was thinking tbh :D Although I think it should compare against both -1 and 0, since neither indicates a valid handle.