Re: [boost] possible problem with boost::iostreams::file_descriptor

Subject: Re: [boost] possible problem with boost::iostreams::file_descriptor
From: Eric MALENFANT (Eric.Malenfant_at_[hidden])
Date: 2009-06-11 09:22:55

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?