Re: [boost] Subject: Formal Review of Proposed Boost.Process library starts tomorrow

Subject: Re: [boost] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Vicente Botet (vicente.botet_at_[hidden])
Date: 2011-02-08 06:02:31

Ilya Sokolov-2 wrote:
> 
> On Tue, 08 Feb 2011 14:22:08 +0500, Vicente Botet  
> <vicente.botet_at_[hidden]> 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  not wrong. 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?
>>>
> 
>> 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
> 

In this case all these interface need to document in which state the native
type need to be. For example, if I'm not wrong, the handle class expect its
native handle to be open, isn't it?

>>>> [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 ).
> 

I would have no problem to accept bp::windows::pipe and  bp::posix::pipe.

>>>> Extract from post [process] Arguments and Context concepts
>>>> http://boost.2283326.n4.nabble.com/process-Arguments-and-Context-concepts-tt3225411.html
>>>>
>>>> [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.
> 

Note that not all the command lines need quotation, and most of the times
they don't need it. At least for this case the set_command_line will
simplify the user task.

>>> 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?
> 

The command_line instance of course. This is the same as string::c_str()
which returns const char* and owns its data.

Best,
Vicente
-- 
View this message in context: http://boost.2283326.n4.nabble.com/Subject-Formal-Review-of-Proposed-Boost-Process-library-starts-tomorrow-tp3263607p3275728.html
Sent from the Boost - Dev mailing list archive at Nabble.com.