On Tue, 08 Feb 2011 14:22:08 +0500, Vicente Botet <vicente.botet@wanadoo.fr> wrote:
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 the implementation?
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 handle?
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 (GetStdHandle()) 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.
Why?
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.
No
[pipe] pipe is defined like
typedef boost::asio::posix::stream_descriptor pipe;
or
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 should 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 http://boost.2283326.n4.nabble.com/process-Arguments-and-Context-concepts-tt...
[snip]
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.
[snip]
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 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.
Yes.
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 as 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?