Bash: incomplete return value

Question

I am trying to create a bash function so that I can call on it from various parts of my code.

#!/bin/bash

host='10.9.8.14'
if [ -z $host ]; then
        echo "Usage: `basename $0` [HOST]"
        exit 1
fi

ping_func () {
        results=`ping -W 1 -c 1 $host | grep 'bytes from '`
        return results
}
while :; do
        result=ping_func
#       result=`ping -W 1 -c 1 $host | grep 'bytes from '`
        if [ $? -gt 0 ]; then
                echo -e "`date +'%Y/%m/%d %H:%M:%S'` - host $host is \033[0;31mdown\033[0m"
                for i in `seq 1 10`;
                do
                        echo $i
                done
                if [ $i -eq 10 ]; then
                        service openvpn restart > /dev/null 2>&1
                        sleep 5
                fi
        else
                echo -e "`date +'%Y/%m/%d %H:%M:%S'` - host $host is \033[0;32mok\033[0m -`echo $result | cut -d ':' -f 2`"
                sleep 1 # avoid ping rain
        fi
done

But when i call the function ping_func from within the loop, my output is as follows:

2019/06/29 08:15:38 - host 10.9.8.14 is ok -
2019/06/29 08:15:40 - host 10.9.8.14 is ok -
2019/06/29 08:15:42 - host 10.9.8.14 is ok -
2019/06/29 08:15:44 - host 10.9.8.14 is ok -
2019/06/29 08:15:46 - host 10.9.8.14 is ok -
2019/06/29 08:15:48 - host 10.9.8.14 is ok -
2019/06/29 08:15:50 - host 10.9.8.14 is ok -

Without the function, but calling ping every loop cycle, I get the correct output.

2019/06/29 08:15:26 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=414 ms
2019/06/29 08:15:27 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=407 ms
2019/06/29 08:15:29 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=410 ms
2019/06/29 08:15:30 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=412 ms
2019/06/29 08:15:31 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=358 ms
2019/06/29 08:15:33 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=466 ms
2019/06/29 08:15:34 - host 10.9.8.14 is ok - icmp_seq=1 ttl=64 time=407 ms

How do I call on my bash function and return all the string data output?

Lines of code in question:

    result=ping_func
    result=`ping -W 1 -c 1 $host | grep 'bytes from '`

`if [ -z "$host" ]; then echo "Usage: $(basename $0) [HOST]"; exit 1; fi` is not good practice. If you exit 1, the message printed is presumed to be an error and ought to be written to stderr (`echo ... >&2)`. OTOH, if you consider the message to be informational and not an error, then you ought to `exit 0`. — William Pursell, Jun 29 '19 at 13:34
"return value" is a single-byte number. What you're talking about is the *stdout*. (Similarly, `return results` makes no sense at all, as you can't return a string, you can only return a number). `foo=$(bar)` doesn't capture what `bar` *returns*, it captures what `bar` *outputs*. — Charles Duffy, Jun 29 '19 at 13:35
BTW, to clarify what @WilliamPursell was saying -- stderr isn't *limited* to errors, but all informational and diagnostic content should be written there; error messages are definitely diagnostic in nature. (So are regular logs, unless your program is written for the purpose of displaying logs, in which case they're intended output vs diagnostic content and should be written to stdout). — Charles Duffy, Jun 29 '19 at 13:37
As another side, calling `date` as an external command is really high-overhead -- you're probably adding tens of milliseconds of execution time to each end of your loop. Much more efficient, if targeting bash 4.3 or newer, to use `printf '%(%Y%m%d %H:%M:%S)T %s\n' -1 "$result"` instead. — Charles Duffy, Jun 29 '19 at 13:41

Charles Duffy · Accepted Answer · 2019-06-29T13:56:58.833

Your code isn't actually starting the function at all; result=ping_func isn't how you assign the output of running ping_func to result (and even if it were, return isn't how you emit something from a function as output).

#!/usr/bin/env bash
host=${1:-10.9.8.14}

# a regular expression to pull out the line we want; tune to taste
# this is internal to bash, so much faster than calling grep/sed/cut/etc once
# for each line!
content_re='[Ff]rom.*:[[:space:]]+([[:print:]]*)'
ping_func() {
    local output retval  # declare your locals so we don't leak scope

    # actually call ping; store its output and success/failure state separately
    output=$(ping -W 1 -c 1 "$1"); retval=$?

    # compare against content_re as a regex; far faster than grep
    [[ $output =~ $content_re ]] && printf '%s\n' "${BASH_REMATCH[1]}"

    # return the exit status we stored earlier as our own
    return "$retval"
}

while :; do
    # the if branches on whether the returned retval was 0
    # ...whereas the stdout written by printf is put in result
    if result=$(ping_func "$host"); then
        # using printf %(...)T needs a new bash, but is *far* faster than date
        printf '%(%Y%m%d %H:%M:%S)T Host is UP:   %s\n' -1 "$result"
    else
        printf '%(%Y%m%d %H:%M:%S)T Host is DOWN: %s\n' -1 "$result"
    fi
    sleep 1
done

That said, I'd never write code like this myself: Better to run one long-running copy of ping and handle each line of output it writes separately, so you don't keep starting ping over and over. See BashFAQ #1 for guidance.

Bash: incomplete return value

1 Answers1