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

Subject: Re: [boost] possible problem with boost::iostreams::file_descriptor
From: Zachary Turner (divisortheory_at_[hidden])
Date: 2009-06-11 09:32:38

On Thu, Jun 11, 2009 at 8:22 AM, Eric MALENFANT <
Eric.Malenfant_at_[hidden]> 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.