Bash Script not detecting failed exit codes

Question

I can't get my bash script (a logging file) to detect any other exit code other than 0, so the count for failed commands isn't being incremented, but the successes is incremented regardless of whether the command failed or succeeded.

Here is the code:

#!/bin/bash
#Script for Homework 8
#Created by Greg Kendall on 5/10/2016
file=$$.cmd
signal() {
    rm -f $file
    echo
    echo "User Aborted by Control-C"
    exit
}
trap signal 2
i=0
success=0
fail=0
commands=0
read  -p "$(pwd)$" "command"
while [ "$command" != 'exit' ]
do
    $command
    ((i++))
    echo $i: "$command" >> $file
    if [ "$?" -eq 0 ]
        then
            ((success++))
            ((commands++))
        else
            ((fail++))
            ((commands++))
    fi
    read -p "$(pwd)" "command"
done
if [ "$command" == 'exit' ]
    then
    rm -f $file
    echo commands:$commands "(successes:$success, failures:$fail)"
fi

Any help would be greatly appreciated. Thanks!

This is why it's bad practice to check $? when you can just do if $command; then ... and observe exit status directly — Charles Duffy
– Charles Duffy, Commented May 11, 2016 at 2:21
that said, putting your command in a scalar variable is full of bugs too; see mywiki.wooledge.org/BashFAQ/050 — Charles Duffy
– Charles Duffy, Commented May 11, 2016 at 2:21
also, == isn't a valid POSIX string comparison operator -- you're thinking of =. The former is supported as an extension in the test builtin provided by some common operating systems and shells, but it's not guaranteed to be present. — Charles Duffy
– Charles Duffy, Commented May 11, 2016 at 2:22
...btw, try running this whole thing through shellcheck.net and fixing what it finds. — Charles Duffy
– Charles Duffy, Commented May 11, 2016 at 2:24

Community · Accepted Answer · 2017-05-23 12:31:41Z

3

That's because echo $i: "$command" is succeeding always.

The exit status $? in if [ "$?" -eq 0 ] is actually the exit status of echo, the command that is run immediately before the checking.

So do the test immediate after the command:

$command
if [ "$?" -eq 0 ]

and use echo elsewhere

Or if you prefer you don't need the $? check at all, you can run the command and check status within if alone:

if $command; then .....; else ....; fi

If you do not want to get the STDOUT and STDERR:

if $command &>/dev/null; then .....; else ....; fi

** Note that, as @Charles Duffy mentioned in the comment, you should not run command(s) from variables.

edited May 23, 2017 at 12:31

CommunityBot

11 silver badge

answered May 11, 2016 at 2:18

heemayl

42.5k10 gold badges86 silver badges87 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Charles Duffy Over a year ago

My only concern here is that echoing the OP's use of $command could be taken to endorse this as an acceptable practice. At barest minimum it should be eval "$command" -- that's not good practice, but at least it works (vs the current form, which fails w/ any command having quotes, backslashes, etc).

John1024 · Accepted Answer · 2016-05-11 02:38:47Z

0

Your code is correctly counting the number of times that the echo $i: "$command" command fails. I presume that you would prefer to count the number of times that $command fails. In that case, replace:

$command
((i++))
echo $i: "$command" >> $file
if [ "$?" -eq 0 ]

With:

$command
code=$?
((i++))
echo $i: "$command" >> $file
if [ "$code" -eq 0 ]

Since $? captures the exit code of the previous command, it should be placed immediately after the command whose code we want to capture.

Improvement

To make sure that the value of $? is captured before any other command is run, Charles Duffy suggests placing the assignment on the same line as the command like so:

$command; code=$?
((i++))
echo $i: "$command" >> $file
if [ "$code" -eq 0 ]

This should make it less likely that any future changes to the code would separate the command from the capture of the value of $?.

edited May 11, 2016 at 2:38

answered May 11, 2016 at 2:15

John1024

115k15 gold badges152 silver badges183 bronze badges

5 Comments

Greg Kendall Over a year ago

Thanks! Just a question though, why does using another variable in place of $? make a difference?

Charles Duffy Over a year ago

@GregKendall, what makes the difference is that the value of $? is being stored immediately after $command runs, so it's not being changed by your (( i++ )) or your echo.

Charles Duffy Over a year ago

BTW, I tend to suggest putting the capture on the same line as the command: your_command; code=$? -- that way other commands (like echo) won't be put between the command and capture by someone else adding debug logging or such and not looking too closely.

John1024 Over a year ago

@CharlesDuffy Excellent suggestion. Answer updated with that change.

Charles Duffy Over a year ago

Good deal. btw, as I mentioned to heemayl, consider eval "$command" vs $command -- that way we aren't hitting bugs when the command contains quotes, backslashes, or likewise.

Collectives™ on Stack Overflow

Bash Script not detecting failed exit codes

2 Answers 2

1 Comment

Improvement

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Improvement

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related