Re: [boost] Presentation of Boost.Process (SoC project)

From: Julio M. Merino Vidal (jmmv84_at_[hidden])
Date: 2006-08-23 04:25:03

On 8/23/06, Oleg Abrosimov <beholder_at_[hidden]> wrote:
> Julio M. Merino Vidal wrote:
> >> 3) seems that you need to fix you quickbook code:
> >> // quickbook:begin(command-line)
> >> bp::command_line cl("svn");
> >> cl.argument("update");
> >> // quickbook:end(command-line)
> >
> > What do you mean?  I can't see the problem there.
>
> this code snippet is from the library docs. such a comment adds nothing
> here (it can not improve understanding of code), furthermore, it
> clutters code and makes it less readable. That's why I've concluded that
> it is a quickbook misconfiguration.

Ah, I see... that's a misfeature in Quickbook.  That block of code is
nested in another quickbook:begin/end section; when including the
outer one in the document, the inner markers are not removed.  I guess
I could modify my little patch to Quickbook to bypass them; it's easy
:-)

> Now I've realized that this stuff is not needed at all. It is used to
> construct the string that you pass to function shell() (I've simplified
> things a bit). Alternatively, you could remove arguments stuff and
> provide only the shell() function and leave a user with constructing the
> arguments string by itself. The reason is that if you are going to
> provide a way to simplify the process - you are trying to do too much.
> It is not an easy task to create a sub-library to manipulate program
> arguments that would satisfy the majority of us. For example, what about
> not only adding but also removing arguments and maybe replacing? To
> conclude - it is a separate task to create an arguments manipulating
> sub-library. You should better don't do it now. May be later...

Well... there is no way I remove the possibility to construct a
command line argument by argument.  As mentioned in the documentation,
this is the safest way to guarantee that the arguments passed are what
will really end up in the program's argv[].

But I understand your point.  A possibility could be to simply bypass
the command_line class and turn it into a regular collection -- e.g. a
vector.  We'd even have:

typedef std::vector< std::string > command_line;

or something like that.

shell() could then be a free function that constructs such a vector.
And the user could be free to construct the vector on his own if he
wanted to with whichever technique he wanted.

> > The current getters check for several sanity conditions (currently
> > using assertions, but exceptions should be considered).  Moving to
> > plain attributes means that these cannot take place any more :(
>
> it suggests that you have an invariant here and your checks ensure it.
> It can be a significant improvement if this hidden invariant becomes
> visible and explicit in user code. in this case you'll have no need to
> do any sanity checks (it is not always possible, though). For example,
> if object obj can be used only after function foo was called you can
> enforce it by returning the obj from foo. and prohibit any other way to
> obtain an obj.

Aha.

> >
> > I'm assuming that you'd store all the status on the 'status' class,
> > and its children could only provide getters for additional
> > information, right?  If so, sounds better than the template approach
> > because it's easier to use.
> >
> > The only "strange" situation that can arise is this:
> >
> > bp::status s = c.wait();
> >
> > if (s.exited())
> >    // OK, deal with exit status.
> > else
> >    // Uh... something happened but we cannot know exactly what.  Why?
> >
> > (I'm only afraid of people trying to use exit_status() when exited()
> > returns false in this specific situation because they'd see that
> > exited() never returns true under Windows...)
>
> The idea was that bp::status has all the information, but exposes
> publicly only the portable part. in order to obtain platform-specific
> info one needs to construct the platform specific status object and
> access it's member functions. For example, because status::exited() is
> not useful under win32 bp::status wouldn't contain this method.

exited() is useful to ensure that exit_status() is valid.  Directly
calling exit_status() (without checking exited() first) could not be
portable because it could return bogus values under POSIX.  See below:

> >> where exit_success is a const status object or object of any other
> >> special type (to avoid unsafe comparisons to int in case of using enum)
> >
> > Indeed.  See reply to 2 ;-)
>
> boost::tribool can be used to eliminate exited() function and all ugly
> comparisons with EXIT_SUCCESS/EXIT_FAILURE

But the problem is that EXIT_SUCCESS and EXIT_FAILURE are *not* the
only possible values for the exit code.  A program returns an integer;
in fact, there are many programs that return several integers (not
only 1 = EXIT_FAILURE) to denote different errors.

Or you mean to turn exited() into a tribool while still keeping
exit_status()?  The "problem" is that signaled() and stopped() could
behave differently...

By the way, I've changed the code as somebody else suggested: there is
now a posix_status class and the old status now only provides exited()
and exit_status().

Thank you!

-- 
Julio M. Merino Vidal <jmmv84_at_[hidden]>
The Julipedia - http://julipedia.blogspot.com/