develooper Front page | perl.perl5.porters | Postings from October 2018

[perl #133509] IPC::Open2 and IPC::Open3 documentation updates

Thread Next
From:
James E Keenan via RT
Date:
October 9, 2018 14:28
Subject:
[perl #133509] IPC::Open2 and IPC::Open3 documentation updates
Message ID:
rt-4.0.24-1289-1539095297-1642.133509-15-0@perl.org
On Mon, 10 Sep 2018 22:18:06 GMT, grinnz@gmail.com wrote:
> Sorry... one more time.

There is a lot to this patch.  In what follows I'm responding only to the sections dealing with IPC::Open2.

> diff --git a/ext/IPC-Open3/lib/IPC/Open2.pm b/ext/IPC-Open3/lib/IPC/Open2.pm
> index 9e27144571..9c70e5b455 100644
> --- a/ext/IPC-Open3/lib/IPC/Open2.pm
> +++ b/ext/IPC-Open3/lib/IPC/Open2.pm
> @@ -18,38 +18,41 @@ IPC::Open2 - open a process for both reading and writing using open2()
>  
>      use IPC::Open2;
>  
> -    $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'some cmd and args');
> -      # or without using the shell
> -    $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'some', 'cmd', 'and', 'args');
> +    my $pid = open2(my $chld_out, my $chld_in, 'some', 'cmd', 'and', 'args');
> +    # or passing the command through the shell
> +    my $pid = open2(my $chld_out, my $chld_in, 'some cmd and args');

In the section above you have reversed the order in which the variants are
presented, i.e., from "shell-first" to "shell-second".  Was that intentional?
(It made reviewing the patch a bit more difficult for me.)

>  
> -    # or with handle autovivification
> -    my($chld_out, $chld_in);
> -    $pid = open2($chld_out, $chld_in, 'some cmd and args');
> -      # or without using the shell
> -    $pid = open2($chld_out, $chld_in, 'some', 'cmd', 'and', 'args');
> +    # read from parent STDIN and write to already open handle
> +    open my $outfile, '>', 'outfile.txt' or die "open failed: $!";
> +    my $pid = open2($outfile, '<&STDIN', 'some', 'cmd', 'and', 'args');
>  
> +    # read from already open handle and write to parent STDOUT
> +    open my $infile, '<', 'infile.txt' or die "open failed: $!";
> +    my $pid = open2('>&STDOUT', $infile, 'some', 'cmd', 'and', 'args');
> +

In the section above you have removed the "using shell" example.  Was that
intentional?

> +    # reap zombie and retrieve exit status
>      waitpid( $pid, 0 );
>      my $child_exit_status = $? >> 8;
>  
>  =head1 DESCRIPTION
>  
> -The open2() function runs the given $cmd and connects $chld_out for
> +The open2() function runs the given command and connects $chld_out for
>  reading and $chld_in for writing.  It's what you think should work 
>  when you try
>  

I think your substitution of 'command' for '$cmd' is good, because what the
doc is trying to do here is to provide an overall description of the
functionality.  I would recommend taking that one step farther:  replacing
$chld_out and $chld_in with descriptions of those two filehandles which don't
depend on the variable names used in the SYNOPSIS.

> -    $pid = open(HANDLE, "|cmd args|");
> +    my $pid = open(my $fh, "|cmd args|");
>  
> -The write filehandle will have autoflush turned on.
> +The $chld_in filehandle will have autoflush turned on.
>  

I think it would be better if you left "write filehandle" in, then went on to
provide a code snippet that illustrates that $chld_in is the write filehandle.

Also, shouldn't '$chld_in' be 'C<$chld_in>'?

>  If $chld_out is a string (that is, a bareword filehandle rather than a glob
>  or a reference) and it begins with C<< >& >>, then the child will send output
>  directly to that file handle.  If $chld_in is a string that begins with
>  C<< <& >>, then $chld_in will be closed in the parent, and the child will
> -read from it directly.  In both cases, there will be a dup(2) instead of a
> -pipe(2) made.
> +read from it directly.  In both cases, there will be a L<dup(2)> instead of a
> +L<pipe(2)> made.
>  
> -If either reader or writer is the null string, this will be replaced
> -by an autogenerated filehandle.  If so, you must pass a valid lvalue
> +If either reader or writer is the empty string or undefined, this will be
> +replaced by an autogenerated filehandle.  If so, you must pass a valid lvalue
>  in the parameter slot so it can be overwritten in the caller, or
>  an exception will be raised.
>  
> @@ -66,10 +69,10 @@ Failing to do this can result in an accumulation of defunct or "zombie"
>  processes.  See L<perlfunc/waitpid> for more information.
>  
>  This whole affair is quite dangerous, as you may block forever.  It
> -assumes it's going to talk to something like B<bc>, both writing
> +assumes it's going to talk to something like L<bc(1)>, both writing
>  to it and reading from it.  This is presumably safe because you
> -"know" that commands like B<bc> will read a line at a time and
> -output a line at a time.  Programs like B<sort> that read their
> +"know" that commands like L<bc(1)> will read a line at a time and
> +output a line at a time.  Programs like L<sort(1)> that read their
>  entire input stream first, however, are quite apt to cause deadlock.
>  
>  The big problem with this approach is that if you don't have control 
> @@ -77,8 +80,8 @@ over source code being run in the child process, you can't control
>  what it does with pipe buffering.  Thus you can't just open a pipe to
>  C<cat -v> and continually read and write a line from it.
>  
> -The IO::Pty and Expect modules from CPAN can help with this, as they
> -provide a real tty (well, a pseudo-tty, actually), which gets you
> +The L<IO::Pty> and L<Expect> modules from CPAN can help with this, as
> +they provide a real tty (well, a pseudo-tty, actually), which gets you
>  back to line buffering in the invoked command again.
>  
>  =head1 WARNING 

-- 
James E Keenan (jkeenan@cpan.org)

---
via perlbug:  queue: perl5 status: new
https://rt.perl.org/Ticket/Display.html?id=133509

Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About