56

I want to write code like this:

command="some command"

safeRunCommand $command

safeRunCommand() {
   cmnd=$1

   $($cmnd)

   if [ $? != 0 ]; then
      printf "Error when executing command: '$command'"
      exit $ERROR_CODE
   fi
}

But this code does not work the way I want. Where did I make the mistake?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Kolesar
  • 1,265
  • 3
  • 19
  • 41

5 Answers5

74

Below is the fixed code:

#!/bin/ksh
safeRunCommand() {
  typeset cmnd="$*"
  typeset ret_code

  echo cmnd=$cmnd
  eval $cmnd
  ret_code=$?
  if [ $ret_code != 0 ]; then
    printf "Error: [%d] when executing command: '$cmnd'" $ret_code
    exit $ret_code
  fi
}

command="ls -l | grep p"
safeRunCommand "$command"

Now if you look into this code, the few things that I changed are:

  • use of typeset is not necessary, but it is a good practice. It makes cmnd and ret_code local to safeRunCommand
  • use of ret_code is not necessary, but it is a good practice to store the return code in some variable (and store it ASAP), so that you can use it later like I did in printf "Error: [%d] when executing command: '$command'" $ret_code
  • pass the command with quotes surrounding the command like safeRunCommand "$command". If you don’t then cmnd will get only the value ls and not ls -l. And it is even more important if your command contains pipes.
  • you can use typeset cmnd="$*" instead of typeset cmnd="$1" if you want to keep the spaces. You can try with both depending upon how complex is your command argument.
  • 'eval' is used to evaluate so that a command containing pipes can work fine

Note: Do remember some commands give 1 as the return code even though there isn't any error like grep. If grep found something it will return 0, else 1.

I had tested with KornShell and Bash. And it worked fine. Let me know if you face issues running this.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
havexz
  • 9,550
  • 2
  • 33
  • 29
  • You can see the `$command` is being used in the `safeRunCommand`'s `printf` statement. You can use it as it is global. So I am going to change `$command` to `$cmnd` (the local variable.. – havexz Nov 23 '11 at 16:25
  • Thanks @havexz. How can I run command like as: `command="ls -l | grep *.log"` Unfortunately this command is not work, and all my commands are very complex with many pipe |, grep, awk, sed, ... – Kolesar Nov 23 '11 at 20:27
  • Updated the code and only kept the one soln which is more robust. – havexz Nov 23 '11 at 23:17
  • One more question :) How I can add output of the function `safeRunCommand` to variable, and in case of failure to process interrupts If I write `output=$(safeRunCommand "$command")` script is not being interrupt on error inside safeRunCommand function. – Kolesar Nov 24 '11 at 15:10
  • Maybe I found answer. Can I write `out=$(eval $cmnd)`, and after calling `safeRunCommand` function use `out` variable? – Kolesar Nov 24 '11 at 15:43
  • what exactly are you trying to achieve? – havexz Nov 24 '11 at 18:49
  • I making some kind of framework that contains a lot of scripts and a lot of commands. I want to make a function that I call whenever I want to make a command. Whether it is 'usually' command or a command that return any output which I will later parse, check, ... – Kolesar Nov 24 '11 at 20:45
  • Generally you give the user full freedom to run his/her command and after that they are suppose to call an api like `CheckReturnCode` which will just contain error related code from above. As some times commands need to redirect the output and may take input from tty so its every difficult to cover all those scenarios... – havexz Nov 29 '11 at 03:04
8

Try

safeRunCommand() {
   "$@"

   if [ $? != 0 ]; then
      printf "Error when executing command: '$1'"
      exit $ERROR_CODE
   fi
}
sehe
  • 374,641
  • 47
  • 450
  • 633
3

It should be $cmd instead of $($cmd). It works fine with that on my box.

Your script works only for one-word commands, like ls. It will not work for "ls cpp". For this to work, replace cmd="$1"; $cmd with "$@". And, do not run your script as command="some cmd"; safeRun command. Run it as safeRun some cmd.

Also, when you have to debug your Bash scripts, execute with '-x' flag. [bash -x s.sh].

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
  • To quickly elaborate on why this is correct: `$($cmd)` will execute `$cmd` in a *new* shell, whereas `$cmd` won't. – gamen Nov 21 '11 at 14:46
  • `cmd="$@"` will not work; instead, either run `"$@"` directly (as in @sehe's answer), or use an array: `cmd=("$@"); "${cmd[@]}"` – Gordon Davisson Nov 21 '11 at 16:22
  • @GordonDavisson: I try on my system before posting anything here. And it is working fine. Here is output on bash -x try.sh . `+ command='ls java' + safeRunCommand ls java + cmnd='ls java' + ls java ` . It is same as doing cmd="hello world", isn't it? – Priyank Bhatnagar Nov 21 '11 at 17:35
  • 1
    @logic_max: try it on a command with a space in an argument, like `safeRunCommand grep "this that" file.txt`: `+ cmd='grep this that file.txt'` `+ grep this that file.txt` `grep: that: No such file or directory` – Gordon Davisson Nov 21 '11 at 20:00
  • @GordonDavisson: Yes, i get it. But, then he shouldn't run the script as `command="some command"`. – Priyank Bhatnagar Nov 22 '11 at 05:22
  • 1
    @logic_max: Yup. I'm not sure why people insist on storing commands in variables before executing them; it just created limitations like this. – Gordon Davisson Nov 22 '11 at 07:10
1

The normal idea would be to run the command and then use $? to get the exit code. However, sometimes you have multiple cases in which you need to get the exit code. For example, you might need to hide its output, but still return the exit code, or print both the exit code and the output.

ec() { [[ "$1" == "-h" ]] && { shift && eval $* > /dev/null 2>&1; ec=$?; echo $ec; } || eval $*; ec=$?; }

This will give you the option to suppress the output of the command you want the exit code for. When the output is suppressed for the command, the exit code will directly be returned by the function.

I personally like to put this function in my .bashrc file.

Below I demonstrate a few ways in which you can use this:


# In this example, the output for the command will be
# normally displayed, and the exit code will be stored
# in the variable $ec.

$ ec echo test
test
$ echo $ec
0

# In this example, the exit code is output
# and the output of the command passed
# to the `ec` function is suppressed.

$ echo "Exit Code: $(ec -h echo test)"
Exit Code: 0

# In this example, the output of the command
# passed to the `ec` function is suppressed
# and the exit code is stored in `$ec`

$ ec -h echo test
$ echo $ec
0

Solution to your code using this function

#!/bin/bash
if [[ "$(ec -h 'ls -l | grep p')" != "0" ]]; then
    echo "Error when executing command: 'grep p' [$ec]"
    exit $ec;
fi

You should also note that the exit code you will be seeing will be for the grep command that's being run, as it is the last command being executed. Not the ls.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Nathan F.
  • 3,250
  • 3
  • 35
  • 69
1

There are several things wrong with your script.

Functions (subroutines) should be declared before attempting to call them. You probably want to return() but not exit() from your subroutine to allow the calling block to test the success or failure of a particular command. That aside, you don't capture 'ERROR_CODE' so that is always zero (undefined).

It's good practice to surround your variable references with curly braces, too. Your code might look like:

#!/bin/sh
command="/bin/date -u"          #...Example Only

safeRunCommand() {
   cmnd="$@"                    #...insure whitespace passed and preserved
   $cmnd
   ERROR_CODE=$?                #...so we have it for the command we want
   if [ ${ERROR_CODE} != 0 ]; then
      printf "Error when executing command: '${command}'\n"
      exit ${ERROR_CODE}        #...consider 'return()' here
   fi
}

safeRunCommand $command
command="cp"
safeRunCommand $command
JRFerguson
  • 7,426
  • 2
  • 32
  • 36
  • `cmnd="$@"` will not work; instead, either run `"$@"` directly (as in @sehe's answer), or use an array: `cmnd=("$@"); "${cmnd[@]}"` – Gordon Davisson Nov 21 '11 at 16:23