http://qs321.pair.com?node_id=146445


in reply to Re: Open3 and bad gut feeling : 357 char STDIN limit?
in thread Open3 and bad gut feeling

Welp.. turns out that if I pass the command+args as an array instead of a string to Open3 the problem disapears. I don't even have to create an script to run large command lines.

The DOS problem was eliminated by slurping up the args via set VAR=%*.

So to put an end to this here's the code as I am going to run with it.

# RETURNS: %OSexec = ( stdout => $stdout, stderr => $stderr pid => $pi +d ) sub OSexecute { my @execute = @_; my $stdin = IO::Select->new(); # not sure what difference <&STDIN and \*STDIN have $stdin->add("\*STDIN"); my $din = IO::Handle->new(); $din->autoflush(1); $stdin->add($din); my $stdout = IO::Select->new(); $stdout->add(\*STDOUT); my $dout = IO::Handle->new(); $dout->autoflush(1); $stdout->add($dout); my $stderr = IO::Select->new(); $stderr->add(\*STDERR); my $derr = IO::Handle->new(); $derr->autoflush(1); $stderr->add($derr); my ( $pid, $val); my $debug = "false"; #$debug = "true"; if ( $debug =~ /true/i ) { print "OSify::Execute::OSexecute::\$execute = @execute\n"; } # Here is the actual execution eval { $pid = open3( $din, $dout, $derr, @execute ); # waitpid waits for the proces to exit # $val could be used as a means to determine status # while waiting if that functionality becomes needed. $val = waitpid(-1,0); #wait's for process to complete }; $@ && die "OSify::OSexecute died upon execution of\n@execute\nWith +$@"; # Gather the results my $line; # Standard Out my @stdout = <$dout>; my $out; foreach $line (@stdout) { $line = OSify::Utility::trimSpaces($line); $out = $out . $line; } if ( ! $out ) { $out = 1; } # Standard Error my @stderr = <$derr>; my $err; foreach $line ( @stderr ) { $line = OSify::Utility::trimSpaces($line); $err = $err . $line; } if ( ! $err ) { $err = 1; } # Flush and close the Filehandles $din->flush(); $din->close; $dout->flush(); $dout->close; $derr->flush(); $derr->close; my %OSexec = ( stdout => $out, stderr => $err, pid => $pid, ); return %OSexec; }
coreolyn

Replies are listed 'Best First'.
Re^3: Open3 and bad gut feeling : Finally
by ikegami (Patriarch) on Sep 15, 2004 at 00:30 UTC

    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.

      Big Sincere Thank You for your comments and thoughts on this node!

      As I mentioned earlier I had no problems with it and have exipirienced no issues other than the 'hang' on NT. It has come up a couple of times in the last couple of years but rerunning the base app has worked on both occasions, and I've always had $stderr returned.

      (As for returning the hash... I created a logging scheme ( Prior to log4 pardigms that worked with the hash as a parameter)

      I will make some time to dig back into this now that I've finally gotten somethings to ponder and consider.