Subject: Re: [boost] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2011-02-08 03:30:11
On Tue, 08 Feb 2011 00:09:25 +0500, Vicente Botet
> handle is more than RAII, it tracks shared ownership to the native handle
> but this is not stated clearly on the doc. The sentence "user needs to
> release() to transfer ownership" lets think to some kind of move
> Does it means that the access to handles that had shared ownership will
> access to an invalid handle after ownership has been transferred? It's
> if we had a shared_ptr<unique_ptr<>>. Do we really need shared ownership?
> Isn't unique ownership enough?
It is enough, I think, but we still don't have move-enabled containers.
> What is behind a handle, a process handle or a stream end handle? The
> that the underlying interface manages with non-typed handles, doesn't
> that the C++ interface must follows the un-typed form. We can define a
> strong-type interface to avoid bad uses that will result in run-time
> Until we need to store heterogeneous handles, I would prefer specific
> for each type of handle.
Boost.Process uses bp::handle publicly as file (descriptor/handle) only.
Could we rename it to 'file' then?
> Shouldn't the public constructor from a native_handle be reserved to the
> What about using the bool idiom conversion instead of valid()?
> If the role of handle is to close the native handle at construction,
> when do
> we need to create handles with dont_close? Who will close the native
Nobody in some cases, and that is not a problem (for the result of
GetStdHandle() function, at least).
> How can I create a process instance? From where I can get the pid_type or
> the handle? I think that these constructors must be protected.
> What about renaming it base_process to avoid ambiguities with the
Or, better yet, rename the namespace with plural 'processes'.
>> From where I can get the constructor pid_type ? I think that this
> constructor must be protected and the create_child function declared
That will increase coupling.
> Which are the operations that can be applied on this type? What about
> renaming it to native_pid_type, so the user see that he is manipulating a
> non portable entity.
I suggest to drop process::id_ member of type pid_type and add
process::native_ of type native_type, typedefed to int/HANDLE;
> typedef pipe async_pipe;
> Does it means that on POSIX systems pipe and async_pipe is the same?
> What about executable_to_program_name?
> pipe is defined like
> typedef boost::asio::posix::stream_descriptor pipe;
> typedef boost::asio::windows::stream_handle pipe;
bp::pipe is documented as 'type definition for stream-based classes
defined by Boost.Asio',
so why it can't be named 'asio_stream'?
> Do these classes provide the same interface? If not it is not good to
> them the same name, by the same reason asio has not done it?
> What about defining an opaque class that accepts only the values
> stdout_id, stderr_id on Windows and more on Posix systems?
> [snip] - I answered this on another reply.
> How self().wait() behaves? Blocks forever? throw exception?
> We have already ::terminate(). Why do we need self().terminate()?
> Doesn't Boost.Filesystem provides already get_work_dir, current_path?
> How do you change to another directory?
> rename get_work_dir to get_working_directory?
> What about Boost.ProgramOptions environment_iterator class instead of
> Is get_instance() really needed? If yes, is it the implementation thread
> The single function that left is get_id(). What about moving it to the
> namespace level and just remove this class?
BTW, bp::self::get_work_dir() relies on BOOST_PROCESS_POSIX_PATH_MAX,
but boost::filesystem::initial_path() doesn't.
> Extract from post [process] Arguments and Context concepts
> Windows users will prefer the set_command_line function.
I doubt it ), IMO one interface is enough.
> User that write portable programs would need to choose between one of the
> two ways to pass the arguments. Of course the program could use some
> conditional compilation that could use the most efficient.
> If the user uses the add interface on Windows, the implementation will
> be as efficient as now.
No need to be efficient here, as all savings will be eaten by the
> If the user uses the set_command_line on c-like systems,
> the class need to tokenize the arguments.
(split_winmain() and split_unix() in boost::program_options)
See my alternative implementation in the attachment.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk