Re: [boost] [boost.process] Review

16 Sep 2016

      On 14 September 2016 at 18:08, Klemens Morgenstern <
klemens.morgenstern@gmx.net> 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.
...
Also here are comments I wrote while reading the current documentation in
...
details:
Thank you very much, that really helps. I commented a few points; but
you're right I should link to more, that will help. Other than that, I'll
work your comments into the documentation.
Good!
...
...
Boost.Process Documentation Review
============================
In reading order:
1. Arguments/Command Style:
   Could you clarify if there are different semantics for cmd and exe/args
styles?
   I assume that it is purely syntactic sugar for C++.
That's actually written there:
"The cmd style will interpret the string as a sequence of the exe and
arguments and parse them as such, while the exe-/args-style will interpret
each string as an argument."
It's actually not syntactic sugar, because the call of CreateProcess is
different on windows. But yeah, I should add this.
4. Starting a process:
...
It is not clear to me what character types are supported by system().
   Also, in general, what are the advantages of using
boost::process::system over std::system?
Uhm, you can path any of the properties. I thought that was obvious by the
examples. It's basically an extended std::system.
Well..

5. I had a hard time finding reference docs to the different names in the
...
...
tutorial,
   because there is no link in the functions and classes names.
6. Does Boost.Process process starting functions allow passing
boost::filesystem::paths or
   anything that looks like it?
You can use this as a parameter for "exe", and for I/O. I.e. you can to
this:
fs::path log="log.txt", out="out.txt", in="in.txt", _exe="stuff.exe";
child c(exe=_exe, std_out>out, std_inlog);
child c2(_exe,    std_out>out, std_inlog);
Those usages are actually mentioned in the doc.
...I guess a lot of my remarks are pointing at some information being
explained latter in the doc, that might have been explained when first
showed
to help with understanding through reading the tutorial.
...
...
8. Why not name the null device null_device instead of null? "null" looks
potentially misleading.
Possibly, but it's shorter. Also, since nullptr is a keyword, what would
it lead you to think?
Btw: I orginally wanted to use nullptr here, but that would lead to
confusion between closing & redirecting to null.
I was suggesting null_device because you describe boost::process::null as
being a null device.
...
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?
...
12. In the example, am I correct that
...
cout << fut.get() << endl;
should be
cout << data.get() << endl;
Yes you are.
15. I would prefer you to clarify in this section that this implementation
...
of terminate() will not
  notify the child process (through C signals on posix or other api on
windows) before ending it,
  and if the user want such notification, point to other some other doc
about this subject.
Oh, well that will be frustrating, because I can only point to a section
where I explain why that's not possible. Would make sense for the review
though.
Yes, I think it's an important think to clarify whatever the way.
...
...
18. The pipe section makes me wonder: if each time you use a synchrounous
pipe you
  enable a potential deadlock which is basically unavoidable, why even
allow it?
  Why not allow only the async_pipe?
That's due to the way the OS are working. The deadlock is not specific to
the library.
It makes sense because you might have an application where boost.asio is
an overkill and you know how the output looks.
I see.
...
22. " It essentially works like a shared_ptr, though several copies may not
...
represent the changes done by one."
  So...it does not work at all like a shared_ptr?
Uhm, yeah, that's bad. It points to the same shared data, but you need to
renew it manually. At least on windows.
Maybe explaining without mentionning shared_ptr would be better, to avoid
confusion.
...
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.
...
...
27. Are there incompatibilities between the different properties? Or are
they all combinable?
Execept for exe-args vs. cmd completely free.
...
29. "The throw_on_error property will enable the exception when launching
a
process. It is unnecessary by default, but may be used, when an additional
error_code is provided."
I do not know at all what this is about. Maybe develop?
Yeah, inherited from process 0.5. It's basically there if you want dual
error handling. This can be useful for further development, but maybe
should not be documented.
...
30. io_service and throw_on_error sections lacks a simple code example.
throw_on_error should be removed and io_service does nothing on it's own.
It's in the asio examples though.
...
31. The I/O syntax needs to be explained. I'm talking about (something &
somethingelse) > text
  Instead of relying on just an example, clarify
    - where this syntax can be used;
    - what should be where (in the parenthesis? are the parenthesis
necessary? what is right/left
      of the arrow?)
    - if it imply some special semantic;
Ok, thanks. This is what you do in the shell "stuff &2> log.txt", so you
combine both out streams to one sink. It's only available for "(stderr &
stdout) > sink".
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.
...
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?
...
--------
...
Note that I didn't try yet to use the library, but intend to before the
review week.
Awesome!
Joël Lamotte
...
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman
/listinfo.cgi/boost
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman
/listinfo.cgi/boost