This code can be trimmed down a lot.
$stdin, $stdout and $stderr are never used, so they should be deleted.
Flushing does nothing for input handles, so $dout->autoflush(1);, $derr->autoflush(1);, $dout->flush(); and $derr->flush(); do nothing.
$din->flush(); is useless for two reason: 1) The program on the other side of the pipe has ended, and 2) the close does a flush.
$din->autoflush(1); is not needed since we don't write to $din. Even if we did, open3 makes it autoflush for us.
Since we don't do anything with $din, $dout and $derr anymore, we can let open3 create them for us.
Filehandles are automatically closed when they go out of scope.
The $pid is for a dead process, so why return it to the caller?
It's more common to use 0/1 for boolean instead of "false"/"true". Deviations make it less readable, and strings make it more complicated. Using a regular expression to check the equality of two strings is overkill. eq is the code for that.
Reformatting $out and $err has nothing to do with OSexecute and should be done by the calling function if it so desires.
There are many small problems.
$stdin->add("\*STDIN"); should be $stdin->add(\*STDIN);.
$val = waitpid(-1, 0); should be $val = waitpid($pid, 0);. $val isn't used, so the assignment can be ommited as well.
"@execute" will cram all the arguments into one word.
Why change a stdout and stderr to 1 if it's empty, when it's just as easy to check for the empty string ($str eq '' and length($str)
$stdout is changed from to 1 if it's '0'. Same for $stderr.
Returning a hash is... odd. If the code did return ($stdout, $stderr);, the caller could very simply do my ($stdout, $stderr) = OSexecute(...);
Cleaned Up Code
# RETURNS: ( $stdout, $stderr ) Both are refs to an array of lines.
sub OSexecute {
my @execute = @_;
local $, = ' '; # for print "...@execute..."
my $debug = 0;
#$debug = 1;
print("OSexecute(@execute)\n") if ($debug);
# Here is the actual execution.
my $pid = eval { open3($din, $dout, $derr, @execute) };
die "OSexecute(@execute): $@" if ($@);
# Wait for process to complete.
waitpid($pid, 0);
# Gather the results
my @stdout = <$dout>;
my @stderr = <$derr>;
# We should check the return code of the child.
# Gotta trap SIGPIPE for that.
return ( \@stdout, \@stderr );
}
There's a big problem
{
my $file_name;
foreach $file_name ('c:\\tinyfile.txt', 'c:\\biggfile.txt') {
my ($stdout, $stderr);
print("$file_name\n", ("=" x length($file_name))."\n", "\n");
($stdout, $stderr)
= OSexecute('cmd.exe', '/c', 'type', "\"$file_name\"");
print("stdout\n", "------\n", @$stdout);
print("Nothing was sent to STDOUT.\n") unless (@$stdout);
print("\n");
print("stderr\n", "------\n", @$stderr);
print("Nothing was sent to STDERR.\n") unless (@$stderr);
print("\n", "\n");
}
}
outputs
c:\tinyfile.txt
===============
stdout
------
foo
bar
bla
stderr
------
Nothing was sent to STDERR.
c:\biggfile.txt
===============
*** HANGS ***
The problem is that file handles (including $dout and $derr) have a buffer that's limited in size. biggfile.txt is less than 2KB, which is really quite small, so this needs to be fixed.
Fix
# RETURNS: ( $stdout, $stderr ) Both are refs to an array of lines.
sub OSexecute {
my @execute = @_;
my @stdout;
my @stderr;
local $, = ' '; # for print "...@execute..."
my $debug = 0;
#$debug = 1;
print("OSexecute(@execute)\n") if ($debug);
# Here is the actual execution.
my $pid = eval { open3($din, $dout, $derr, @execute) };
die "OSexecute(@execute): $@" if ($@);
my $select = IO::Select->new();
$select->add($dout);
$select->add($derr);
my @ready;
my $fh;
# Gather the results
while (@ready = $select->can_read()) {
foreach $fh (@ready) {
push(@stdout, <$fh>) if ($fh == $dout);
push(@stderr, <$fh>) if ($fh == $derr);
}
}
# Wait for process to complete and reap it.
waitpid($pid, 0);
# We should check the return code of the child.
# Gotta trap SIGPIPE for that.
return ( \@stdout, \@stderr );
}
Fixed?
Oops! select() (and IO::Select) doesn't work in Windows. That sucks. It means we're gonna have to use threads!!! I have no experience with threads, so maybe another day.
-
Are you posting in the right place? Check out Where do I post X? to know for sure.
-
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big>
<blockquote> <br /> <dd>
<dl> <dt> <em> <font>
<h1> <h2> <h3> <h4>
<h5> <h6> <hr /> <i>
<li> <nbsp> <ol> <p>
<small> <strike> <strong>
<sub> <sup> <table>
<td> <th> <tr> <tt>
<u> <ul>
-
Snippets of code should be wrapped in
<code> tags not
<pre> tags. In fact, <pre>
tags should generally be avoided. If they must
be used, extreme care should be
taken to ensure that their contents do not
have long lines (<70 chars), in order to prevent
horizontal scrolling (and possible janitor
intervention).
-
Want more info? How to link
or How to display code and escape characters
are good places to start.