Re: [boost] [boost.process] Review

Subject: Re: [boost] [boost.process] Review
From: Klemens Morgenstern (klemens.morgenstern_at_[hidden])
Date: 2016-09-16 15:41:18

Am 16.09.2016 um 14:24 schrieb Klaim - Joël Lamotte:
> On 14 September 2016 at 18:08, Klemens Morgenstern <
> klemens.morgenstern_at_[hidden]> wrote:
>
>> Am 14.09.2016 um 16:29 schrieb Klaim - Joël Lamotte:
>>
>>>
>>>
>>> I share these concerns.
>>>
>>
>> It's not easy to solve, it would probably look like this:
>>
>> child c("äöü", windows::unicode);
>>
>>
> Looks ugly but I'm not specialist in solving this problem
>  Would it be a problem to just allow wstring too, and like
> boost::filesystem, assume it's UTF-16's for window's wide API on Windows?
>
>
>> I do however think, that such cases are rather rare for processes, while
>> they might be regular for filesystems.
>>
>>
> Unfortunately, as soon as you launch processes on windows, you must assume
> that the name of the process
> might have unicode characters, so in general it is better to use the wide
> API (UTF-16 like) anyway.
>
> As allow providing bfs::paths I guess that it's taken care of, but then the
> internals of boost::process which use the windows API must use
> the internal representation of bfs::path (through the native() function)
> which do store pathsin UTF-16.
>
>

Ok, I though that through: as soon as you pass a fs::path on windows, it 
would need to use the unicode version. Which would mean it is stupid not 
to allow wchar_t in general on windwos.
But doing that consequentially, I guess would mean that I would need to 
allow the usage of UTF-16 on posix as well, so I'd need to a feature 
there. And that would then complicate it further, since I might want to 
use a custom codecvt, for whatever reason (but that should be possible).

So what I would endup is something like this:

fs::path thingy{"foo.exe"};
child c(thiny, "-käse", std::codecvt<char16_t, char, std::mbstate_t>());

I'm not sure that's worth it, to be honest.

>
>
>> 9. First example in I/O: maybe I'm wrong but it looks like the last line
>>> can never be read.
>>>    Or maybe I do not understand when exactly does c.running() returns
>>> false.
>>>    Or maybe I'm thinking with windows console that can, if I remember
>>> correctly, continue printing
>>>    output while the command's process is already finished.
>>>
>>> 10. In the same example, isn't there a race between the c.running() and
>>> the
>>> other conditions?
>>>     What if it becomes false after being called, but before calling
>>> getline?
>>>     Same question for the second example.
>>>
>>> I think you're right. I know why I don't use Sync I/O.
>>
>>
> Maybe rewriting it using handlers instead of relying on a racy check would
> be better?
>

Well actually, since I know nm will append a empty line, I can just 
remove the running-check and wait after the I/O is finished.

>> 25. Is there a reason why pwd() and path() doesn't return
>>> boost::filesystem::path objects?
>>>
>>>
>> I already removed them because they're also in boost.filesystem. It did
>> return std::string because it was a const char* from the system.
>>
>> 26. What is a property? When/where can they be used? It is not explained at
>>> all.
>>>   I can guess from the rest of the examples and my experience with other
>>> boost libraries
>>>   but it's still not super clear to the newcomer.
>>>
>>
>> Well a property is something like "exe", a property of the process.
>>
>> Since you brought that up: I didn't want to call it a parameter, because
>> that could get confusing since parameters build properties, i.e.
>> "system("gcc", args="thingy.c", args+="-o", args+="thingy.o");", which
>> would define two properties "exe" and "args". I couldn't call them members
>> though, because they are not visible class-members.
>>
>>
> Yes "property" is a beter name, though it would be clearer if you say that
> it's a type of object that holds a value
> for one of the property of the process.
>
>

Ok, I think I'll write a better definition for that and link it.

>> 32. Why is there 2 different Child sections?
>>>   Why are they separate?
>>>
>>>
>> Because one is the class description, the other is the launch mode.
>>
>>
> Note that it was not completely clear to me because the class description
> is not a reference page
> and both class description and launch mode are in the same page but not all
> the documentation is on this page.
> It looks like you want both a big one page doc and separate some parts,
> which is a bit conflicting when reading the
> doc in order.
>
Thats more to my inapt use of quickbook, not on purpose.
>
>> 33. What is the equivalent process launcher function that the child
>>> constructor's behaviour matches?
>>>
>>>
>> There is none, that's why there's a custom section for this. The child
>> ctor itself does all the work, so I don't see the need to have a launcher
>> function.
>>
>>
> So... if I understood correctly, what you mean is that the other process
> launching functions could be explained in term of the child's constructor.
> Am I correct?
>
>

Well the child-ctor is one way to launch a process, the other two 
functions are different. I.e. the function which launches a process and 
returns a handle IS the child ctor, and only it.