Re: [boost] Boost.Process 0.5 released

Subject: Re: [boost] Boost.Process 0.5 released
From: Roland Bock (rbock_at_[hidden])
Date: 2012-08-21 04:06:49

Hi Boris,

On 2012-08-21 00:11, Boris Schaeling wrote:
> On Sat, 18 Aug 2012 23:54:24 +0200, Roland Bock <rbock_at_[hidden]> wrote:
>
> Hi Roland,
>
>> [...]But things like wait_for_exit seem strange to me: If the library
>> takes
>> care of the forking and handles setting up pipes in such a nice way, why
>> should the user of the library be bothered with the different ways of
>> interpreting the exit information (unless he really, really wants to)?
>
> can you show me some sample code how you'd like to interpret the exit
> code?
For my use case it is totally sufficient to differentiate between
success and failure, so an int would be totally sufficient.

>
>> Same with a /dev/null sink. Offering a factory method would be easy and
>> spares the user.
>
> I think this is a Boost.Iostreams problem. We have a null device in
> Boost.Iostreams (see
> <http://www.boost.org/doc/libs/1_50_0/libs/iostreams/doc/classes/null.html>)
> but it's a class with no-op functions. If the class would open
> /dev/null or NUL, one could use the null device. What I can do for now
> is changing the example in the Boost.Process documentation. Instead of
> writing to /dev/null or NUL, it should write to foo.txt - problem
> solved. ;)

a) Yes, redirecting to foo.txt would make for a nice example as well :-)
b) Hmm. Not following. Why not use the null sink from boost::iostreams?
You can create a stream from it that does what I would expect from
redirecting to /dev/null: Discard all data.

boost::iostreams::stream<boost::iostreams::null_sink>
null((boost::iostreams::null_sink()));

While this is something the library user could figure out on his own, it
would still be a very helpful example, I guess.

BTW: It might also help to explain whether or not you could just add ">
/dev/null" to the argument list (and if you could what the differences
are, for example if one way is expected to be more performant).

c) I would add yet another example by redirecting output to a string.

>> [...]I understand that the signal stuff is not required for Windows,
>> but why
>> not offer a convenience function that sets the signal handler on POSIX
>> and does nothing on Windows?
>>
>> Same with the discard method. Does something on Windows, does nothing on
>> POSIX, and I get to write code without using the #ifdefs every other
>> line.
>
> I'm afraid it leads to code obfuscation if you have a superset of all
> existing functions platforms provide and they are all used together in
> the same code with no clear indication which ones do something useful
> and which ones are no-ops. Imagine you had no-op Windows API functions
> and no-op POSIX API functions. You could use all system API functions
> from Windows and POSIX without #ifdefs in the same source file and
> claim your code is platform-indepedent. But I think I wouldn't call it
> platform-independent but a complete mess. ;) It is maybe no problem
> for the example you are referring to as we are talking only about two
> lines here. But we start trading convenience vs. clarity? And if you
> call discard() and think you are fine on all platforms, you'd make a
> mistake?

You are certainly right that using a wrapper for all individual system
API calls that does something for one system and is a no-op for the
other leads to total confusion.
I am hoping for a way that lets 95% of the users write platform
independent code without the need for #ifdefs.

For example, my idea was to not require the user to call discard() but
have this done in the destructor and the longer I think about it, the
more I like Joel's idea of calling signal to ignore SIGCHLD in the
constructor.

Why? If you say that in POSIX, the typical, the 95% case is to use the
signal call, then I would expect the library to do that for me. And if I
happen to be one of the 5% who don't want to do what most of the others
need, then I need to take special care, and probably use ifdefs.

Thus, maybe you could offer a lightweight child class (as it is right
now) and a RAII child class that does the signal/discard calls.

>
>> The other thing about the discard: I think one of the first things I'd
>> do is write a wrapper for child that is non-copyable (as also suggested
>> by Joel) and calls discard in its destructor. I wonder if that should
>> not be part of the library, too?
>
> This is related to the paragraph above. The RAII type would only make
> sense on Windows but the code would compile on all platforms. If you
> don't know that you must do something extra on POSIX (like ignoring
> SIGCHLD), your program will leak resources (leaking is maybe the wrong
> word as init will clean up after the program exits; but you might be
> tricked into thinking the platform-independent RAII type does
> something for you on all platforms which wouldn't be true).
See above: If 95% of the users would perform the signal call, you could
also make that the standard behavior in the constructor.

>
>> With the probable exception of the asynchronous IO and wait, I think it
>> should be possible to get rid of the #ifdefs in the tutorial. And IMHO
>> that would be a nice improvement for what looks like a cool library
>> already :-)
>
> Thanks! I mentioned it already in another email I think: If I rewrite
> some and remove other examples from the tutorial, most of the #ifdefs
> could disappear. Boost.Process would still support all of that (and I
> know anyway how to do all of that myself :). 
If the #ifdefs are required right now for the given examples, then using
different examples removes a symptom, not the cause. I'd prefer the same
examples.

  * #boost_process.tutorial.cleaning_up_resources
    As written above, I'd offer a second version of the child class
    which does the signal/discard in constructor/destructor.
  * #boost_process.tutorial.setting_up_standard_streams
    Using boost::iostreams::stream<boost::iostreams::null_sink>
    null((boost::iostreams::null_sink())); obsoletes the system
    dependency (and I would add the examples and hints mentioned above)
  * #boost_process.tutorial.asynchronous_i_o
    It might make sense to add such a typedef to boost process for
    convenience. Maybe not. Not sure.
  * #boost_process.tutorial.waiting_for_a_program_to_exit
      o The first #ifdef is hidden in the text: WEXITSTATUS required for
        POSIX? I guess that 90%+ would want to just get the exit code
        and be done with it.
      o I am not sure about the #ifdef in the code. Maybe somebody has a
        nice idea, otherwise I would just let leave it as it is.

Regards,

Roland