0

I am trying to make a simple bash script to check if a ping command could be executed (there is internet) or if not otherwise.

I already had a code which works perfectly which checked directly in the if statement if the ping command can be executed without making an output using /dev/null.

Old code:

#! /usr/bin/bash

YELLOW="\e[33m"
RED="\e[1;31m"
GREEN="\e[1;32m"
ENDCOLOR="\e[0m"

if ping -c 1 8.8.8.8 > /dev/null 2>&1; then
        echo -e "${YELLOW}[*]${ENDCOLOR} You have Internet access."
else
        echo -e "\n ${RED}[!]${ENDCOLOR} You don't have Internet access !"
        echo -e "\n ${YELLOW}[*]${ENDCOLOR} Quitting webDirDiscover..."
        exit 1
fi

Now what I want to do is store the command ping -c 1 8.8.8.8 in a variable called PING. Once this is done, execute the if statement and check the result.

The problem is that doing this returns the error:

./webDirDiscover.sh: line 10: PING: command not found

When I think that the if statement should work correctly.

Current code:

#! /usr/bin/bash

YELLOW="\e[33m"
RED="\e[1;31m"
GREEN="\e[1;32m"
ENDCOLOR="\e[0m"

PING=$(ping -c 1 8.8.8.8)

if $PING -eq 0; then
        echo -e "${YELLOW}[*]${ENDCOLOR} You have Internet access."
else
        echo -e "\n ${RED}[!]${ENDCOLOR} You don't have Internet access !"
        echo -e "\n ${YELLOW}[*]${ENDCOLOR} Quitting webDirDiscover..."
        exit 1
fi

In addition to solving this simple script, this will come in handy when executing more complex commands and thus have a more visual and visually pleasing code.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
RuthIsRoot
  • 135
  • 2
  • 8
  • hi. do not use variables to save commands; from a technical standpoint functions are the apt solution. i will nonetheless share an answer explaining why you the script does not work as expected – kevinnls Jan 04 '22 at 11:51
  • Okay, is there an specific reason to don't use variables to save commands? Right now im leaving work so cannot test doing it with function. When I can i'll try it ! Thx – RuthIsRoot Jan 04 '22 at 11:52
  • 1
    You did not store the command in a variable, you stored the **output** of the command into a variable. Also, what is the point in doing this? If you really want to manipulate a command via a variable (for instance, because you want to fiddle around with parameters in a way which you find too complicated to do in a function), don't store it into a scalar, but into an array (unless you enjoy having debugging nightmares). Of course, you **can** misuse a variable for this purpose, in the same way as you can use a bicycle to transport a heavy oak table. – user1934428 Jan 04 '22 at 12:18

2 Answers2

3

the simple mantra

Variables hold data. Functions hold code. Don't put code inside [string] variables!

source for more info / going further: comprehensive answer on StackExchange


the advanced mantra regarding storing commands in variables is to use arrays as pointed in the comments.


however i personally prefer not to. if at all i am dealing with runtime information, i would use an array only for the arguments, and not for the command itself

i would use functions, as follows

using a function part 1

#!/usr/bin/env bash

# ...
# use descriptive function names
pingGoogleDnsOnce () {
    ping -c 1 8.8.8.8
}

if pingGoogleDnsOnce >/dev/null; then
    # do stuff
else
    # do other stuff
fi

using a function part 2

#!/usr/bin/env bash

# ...
# use descriptive function names
pingGoogleDnsOnce () {
    ping -c 1 8.8.8.8
}
# use descriptive function names
quietlyPingGoogleDnsOnce () {
    pingGoogleDnsOnce >/dev/null
}

if quietlyPingGoogleDnsOnce; then
    # do stuff
else
    # do other stuff
fi

in both these there is no need to "check" anything explicitly in the if-block. the -eq 0 is unnecessary

a defined function can be used like any other command. the exit code of last executed command in the body is the function's exit code.

so if ping fails, the function pingGoogleDnsOnce fails; if ping succeeds, the function succeeds

an exit code of 0 indicates success

and if constructs simply check the exit code of the "command" which follows it, in our case a function.


there is another practice where people check the value of $? - the variable which holds the exit code of the previous command. in such cases the construct is

if [ $? -eq 0 ]; then

BUT THIS IS NOT RECOMMENDED. and placing the command within the if condition is a better approach


why PING=$(ping -c 8.8.8.8) fails

the $(...) construct is called command substitution. it is substituted by the output of the command that is within the (). so the variable PING holds the output of ping...blah blah and the output is a long multiline string that's not a command


kevinnls
  • 728
  • 4
  • 19
  • 1
    "Don't put code inside variables!". I'm not surprised that it came from that blog. That quote is wrong. Bash is a manipulator of commands and storing command arguments which includes `argv[0]` itself is a perfectly valid practice. This has a valid use case in cases where the command to be executed is dynamic. – konsolebox Jan 04 '22 at 12:41
  • 1
    But then you'd want to use an array to take care of quoting etc. See also https://stackoverflow.com/questions/12136948/why-does-shell-ignore-quoting-characters-in-arguments-passed-to-it-through-varia – tripleee Jan 04 '22 at 12:48
  • I implied the use of arrays. The quote originally does not discriminate and I knew people from `#bash` who disliked storing it even in an array. – konsolebox Jan 04 '22 at 13:52
  • meanwhile neither of you noticed that i used `$0` where i meant `$?` excuse me while i writhe in shame – kevinnls Jan 04 '22 at 14:17
  • [**Do not put code in variables.**](https://mywiki.wooledge.org/BashFAQ/050) It will totally work *until you do something that doesn't*, and while the old experts might spot in in seconds, the rest of the world is going to spend days and days debugging and scouring the internet and the medicine cabinet for relief. Build good design habits. You'll be glad you did. When you *have* to put code in vars, consider carefully and own the repercussions. – Paul Hodges Jan 04 '22 at 14:23
  • Learning how to use arrays is necessary to know when to store **command arguments**. If the world find trouble in it, they're only to blame for not learning how to use arrays and quotes properly. Also I'm not saying storing commands in an array should be made a habit. I'm saying that the "absolutism" is absolutely wrong. There are cases where it is useful. Keep your opinion (and that blog) to yourselves. – konsolebox Jan 04 '22 at 14:36
  • @konsolebox touché. i have made changes. i apologise for the biased nature of my answer and thank you for the knowledge i have gained – kevinnls Jan 04 '22 at 14:39
2

Your script attempts to store the output of the command to the PING variable, but not the command itself. There are two basic ways to test the result of a command, and it's either by checking its output or by checking its return code.

In the case of ping, you can simply check if it's successful by checking its return code, so you can do something like this:

if ping -qc1 8.8.8.8 -W1 >/dev/null; then

But if you want to store the command, you can do so by storing its arguments as an array so:

PING=(ping -qc1 8.8.8.8 -W1)

if "${PING[@]}"; then

However, this does not allow you store redirections like >/dev/null, so instead of storing it as an array, better store is a function:

do_ping() {
    ping -qc1 8.8.8.8 -W1 >/dev/null
}

Then you can execute:

if do_ping; then

Beware of using function names that has similar name as its external commands as this would cause circular calls:

ping() {
    ping -qc1 8.8.8.8 -W1 >/dev/null
}

You can avoid it by using the command command:

ping() {
    command ping -qc1 8.8.8.8 -W1 >/dev/null
}
konsolebox
  • 72,135
  • 12
  • 99
  • 105