Error in shell script for detecting packages on system

Question

I wrote a shell script for detecting whether a package is installed or not. My script should write its name and status if it's installed. I can't figure out any problem with my code but when I run it, it doesn't execute the commands under if [ $? == 0 ] condition.

#!/bin/bash

if [ "$1" == "" ]; then
    echo "Please hold the line."
else
    dpkg -s $@ &> /dev/null
fi

if [ $? == 1 ]; then
    echo -e "Package \033[0;31mNOT\033[0m found." >&2
else
    if [ $? == 0 ]; then
        for i in $@; do
            dpkg -s $i | grep Package
            dpkg -s $i | grep Status
        done
    fi
fi

But the most weird thing to me is that it works if I add an echo after if statement. Looks like that:

#!/bin/bash

if [ "$1" == "" ]; then
    echo "Please hold the line."
else
    dpkg -s $@ &> /dev/null
fi

if [ $? == 1 ]; then
    echo -e "Package \033[0;31mNOT\033[0m found." >&2
else
    echo hi
    if [ $? == 0 ]; then
        for i in $@; do
            dpkg -s $i | grep Package
            dpkg -s $i | grep Status
        done
    fi
fi

So if I add an echo -n to right position in my code it will work as I want. But I just want to know what is wrong with first one?

As an aside: since you're using `bash`, you're [better off using `[[` rather than `[` for conditionals](http://stackoverflow.com/a/29320710/45375). If the intent is to remain portable, combining `[` with `==` doesn't make sense, because strictly POSIX-compliant shells only recognize `=`. Finally, `==` (and `=`) are for _string_ comparison; better to use `-eq` for testing _numeric_ equality. — mklement0, Oct 31 '15 at 21:12

score 2 · Answer 1 · edited Oct 31 '15 at 21:08

2

I think in general you could be more deliberate about your return code handling. You are making assumptions about what $? is referring to that may not be valid depending on your program flow, and regardless, make the program harder to read and understand.

#!/bin/bash

dpkg -s $@ &> /dev/null
installed=$?

if [ $installed -eq 0 ]; then
    for i in $@; do
        dpkg -s $i | grep Package
        dpkg -s $i | grep Status
    done
else
    echo -e "Package \033[0;31mNOT\033[0m found." >&2
fi

edited Oct 31 '15 at 21:08

mklement0

382,024
64
607
775

answered Oct 31 '15 at 20:33

Jameson

6,400
6
32
53

Nicely done; to encourage good habits, please double-quote the `$@` instances. – mklement0 Oct 31 '15 at 21:08

Onilton Maciel · Accepted Answer · 2015-11-01T05:23:40.950

$? is the return status of the last executed command. 0 is successful, 1 or anything else is an error. Note:

dpkg -s python &> /dev/null # returns 0 (OK, true) 
# $? equals 0 now

[ #? == 1 ] # false         # returns 1 (error) 
# $? equals 1 now

[ #? == 0 ] # false         # returns 1 (error)

When you put echo, it works:

dpkg -s python &> /dev/null # returns 0 (OK, true) 
# $? equals 0 now

[ #? == 1 ] # false         # returns 1 (error) 
# $? equals 1 now

echo hi                     # returns 0 (OK) 
# $? equals 0 now

[ #? == 0 ] # true          # returns 0 (OK)

You could save $? to a variable, but you don't really need the if inside the else since you already checked if #? == 1 so just put your code inside else:

#!/bin/bash

if [ "$1" == "" ]; then
    echo "Please hold the line."
else
    dpkg -s $@ &> /dev/null
fi

if [ $? == 1 ]; then
    echo -e "Package \033[0;31mNOT\033[0m found." >&2
else
    for i in $@; do
        dpkg -s $i | grep Package
        dpkg -s $i | grep Status
    done
fi

If you are worried of other possible return statuses of $? (greater than one). You could rewrite your script to

#!/bin/bash

if [ "$1" == "" ]; then
    echo "Please hold the line."
else
    dpkg -s $@ &> /dev/null
fi

if [ $? == 0 ]; then
    for i in $@; do
        dpkg -s $i | grep Package
        dpkg -s $i | grep Status
    done
else
    echo -e "Package \033[0;31mNOT\033[0m found." >&2
fi

Error in shell script for detecting packages on system

2 Answers2