Subject: Re: [boost] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2011-02-08 05:33:57
On Tue, 08 Feb 2011 14:22:08 +0500, Vicente Botet
> Ilya Sokolov-2 wrote:
>> Boost.Process uses bp::handle publicly as file (descriptor/handle) only.
>> Could we rename it to 'file' then?
> The handle class is used as parameter of process
as implementation detail
> and stream_ends if I'm notwrong. Couldn't we have two classes so we
> don't use a stream handle to
> construct a process and vice-versa?
I'm fine with it as is, except of the name.
>>> Shouldn't the public constructor from a native_handle be reserved to
> Could you comment this.
IMO no need to be too clever/restrictive.
>>> 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).
> Do you mean that the close of the handle is not really needed in this
> specific case or always?
Not sure what you mean by 'always', but in this specific case
it is strictly not needed.
> we don't have a portable way to get pid_type and the user
> is not intended to use these constructors.
> If this is a public interface,
> the user could be tempted to use them and will need to get the pid_type
> using non portable interfaces,
or portable interfaces written by himself for his own purposes
> which we should avoid.
>>> 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?
Almost (at least async_[read/write]_some are identical)
>>> If not it is not good to give
>>> them the same name, by the same reason asio has not done it?
It is hard to convince windows/posix guy
to name something stream_descriptor/stream_handle, accordingly ).
> Could you comment this? IMHO any class that has a specific interface
> be prefixed with native as the user could need conditional compilation
> that depends on the platform to use it.
I'm fine with or without 'native_' prefix.
>>> 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.
> We disagree here ;-) Why the container based interface would be always
> better than the command_line?
I don't know anybody who wishes to remember quoting rules.
>>> User that write portable programs would need to choose between one of
>>> 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
>> CreateProcess() call.
> You mean we don't need to be efficient here, isn't it? You are right, the
> time taken by the process creation is some order magnitude the time can
> be spared.
>>> 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)
> Do you mean that the user can use these functions in his application,
> using conditional compilation?
Yes, and passing proper separators, quotes, etc to split_unix().
>> See my alternative implementation in the attachment.
> This is better, but it forces the implementation to return the arguments
> a std::vector<std::string>. I would move the functions that make the
> decoding and let the interface return a char**, that is what the
> implementation needs.
> const char** arguments() const
Who will own the result?
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk