Boost C++ Libraries: Ticket #3517: [iostreams] stream closes supplied descriptor

Steven Watanabe — Tue, 15 Jun 2010 03:20:03 GMT

I think the correct fix is to make close() for file_descriptor a no-op, when you pass in an external file descriptor. This makes file_descriptor's semantics simpler, as it either owns the underlying file descriptor or it doesn't.

Daniel James — Tue, 15 Jun 2010 08:10:46 GMT

I think you're right, but I feel cautious about silently changing existing behaviour. I also dislike the boolean parameter. So I'd create a new constructor which takes flags to indicated if the handle should be closed. And maybe close_on_exit if someone wants it. I'd deprecate the existing constructor, so that it's only present if a macro is defined. That way we'd break the compile rather than subtly changing the code. What do you think of something like this? Am I being over-cautious?

namespace boost { namespace iostreams {
enum file_handle_flags {
    close_handle, // maybe owns_handle?
    never_close_handle,
    close_handle_on_exit // Probably shouldn't be public.
};
class file_descriptor {
public:
    typedef char                      char_type;
    typedef [implementation-defined]  category;
    file_descriptor( const std::string& pathname,
                     std::ios_base::open_mode mode =
                         std::ios_base::in | std::ios_base::out );
    file_descriptor( int fd, file_handle_flags);
    // Windows-only
    file_descriptor( HANDLE hFile, file_handle_flags);
// Maybe something with a version number...
#if defined(BOOST_IOSTREAMS_USE_DEPRECATED)
    file_descriptor( int fd, bool close_on_exit = false );
    // Windows-only
    file_descriptor( HANDLE hFile, bool close_on_exit = false );
#endif
    bool is_open() const;
};
} } // End namespace boost::io

It'll be more verbose but I think it's worth it to clear up the ambiguity. Oh, and those constructors should probably have been explicit.

Steven Watanabe — Tue, 15 Jun 2010 13:54:10 GMT

Replying to danieljames:

I think you're right, but I feel cautious about silently changing existing behaviour. I also dislike the boolean parameter. So I'd create a new constructor which takes flags to indicated if the handle should be closed. And maybe close_on_exit if someone wants it. I'd deprecate the existing constructor, so that it's only present if a macro is defined. That way we'd break the compile rather than subtly changing the code. What do you think of something like this?

Sounds good to me.

Am I being over-cautious?

I don't think so.

<snip code>

It'll be more verbose but I think it's worth it to clear up the ambiguity. Oh, and those constructors should probably have been explicit.

Yep.

owner, status, milestone changed

Daniel James — Tue, 15 Jun 2010 20:22:33 GMT

owner changed from Jonathan Turkanis to Daniel James
status new → assigned
milestone Boost 1.41.0 → Boost 1.44.0

Daniel James — Tue, 29 Jun 2010 14:15:17 GMT

(In [63430]) Try to improve file_descriptor's ownership policies. Refs #3517

Deprecate the old 'close_on_exit' constructors.
Introduce new constructors from file handle, taking either never_close_handle or close_handle.
Close current file handle when opening a new file handle, if it would have been closed in the destructor.

status changed; resolution set

Daniel James — Thu, 01 Jul 2010 21:32:36 GMT

status assigned → closed
resolution → fixed

(In [63502]) Merge iostreams.

New constructors/open for file descriptors. Fixes #3517.
Use unique_path instead of tmpnam. Refs #2325.

Daniel James — Tue, 06 Jul 2010 00:55:18 GMT

(In [63679]) Partially merge iostreams.

New constructors/open for file descriptors. Fixes #3517.
Leaving out changes which depend on filesystem v3.