CODEWITHSUNDEEP
perl
MarcoS
MarcoS

Reputation: 13564

executing external command (and reading its output) from a subrotine

I am currently modifying a Perl script that executes several shell commands, and, for some of them, capture the output for further processing. I want to factor out the code that executes the external commands in subroutines. I wrote the following subroutines:

sub execute_command {
  my $cmd = shift;

  Log("executing command $cmd ...");
  system($cmd);
  my $app = ( $? == -1 ) ? $? : $? >> 8;
  if ($app != 0) {
    Log("error executing command $cmd");
    return $FAILURE;
  }
  Log("done");
  return $SUCCESS;
}

sub execute_command_and_get_output {
  my $cmd = shift;

  Log("executing command $cmd ...");
  unless (open( CMD, "$cmd|" )) {
    Log("error executing command $cmd");
    return undef;
  }
  my @cmd = <CMD>;
  close(CMD);
  Log("done");
  return @cmd;    
}

Questions:

  1. execute_command should execute the passed command and return $SUCCESS if everything is OK, or $FAILURE in case of errors. Am I testing $? in the right way?

  2. execute_command_and_get_output should execute the passed command and return the output as an array (containing the output lines); if the execution of the command fails, it should return undef. Is it correct to use unless (open( CMD, "$cmd|" )) { ... } to test for error conditions in the command execution?

Besides answers to my two questions, any suggestion for improvement is appreciated.

Upvotes: 0

Views: 313

Answers (2)

Itamar
Itamar

Reputation: 2151

  • execute_command_and_get_output can return two arguments:

    1. $SUCCESS or $FAILURE
    2. A reference to an array that either has the results or is empty

  • execute_command can also return two arguments:

    1. $SUCCESS or $FAILURE

    2. An error message. For example:

      system ($cmd) == 0 or return ($FAILURE,"error executing command $cmd: $!");
return ($SUCCESS, "done");

  • BTW, you can avoid going through the CMD part using backticks (also, using the same name in more than one context is confusing).

        my @lines = `$cmd`;
    ($? == 0) or return ($FAILURE,"error executing command $cmd: $!");
    return ($SUCCESS, \@lines);

Upvotes: 1

bvr
bvr

Reputation: 9697

1) If you don't care what error has happened, it is enough to check return value of system:

if(system($cmd) == 0) { return $SUCCESS } else { return $FAILURE }

2) Instead of return undef, it is usually better to just return. It works better when called in list context (it is still false). If the return is large, you might want to return reference to avoid copying.

Upvotes: 1

Related Questions