3

I'm writing a small function which compare md512sums of two files. I can't say that I'm good in bash, but I need to return a simple result commented part is long code. It asks for a reduction.

By the way, if someone have any ideas to improve my code, I'll be grateful.

function TestStage()
{
    local URL="distfiles.gentoo.org/releases/${1:7:5}/current-stage3/${1}"
    wget -q ${URL}.DIGESTS
    local SUM_WEB=`cat ${1}.DIGESTS | sed '2q;d'`
    local SUM_LOC=`openssl dgst -r -sha512 ${1}`
    ####### time to return
    return [[ "${SUM_WEB:0:128}" == "${SUM_LOC:0:128}" ]]

    #if [ "${SUM_WEB:0:128}" == "${SUM_LOC:0:128}" ]
    #then
    #    rm ${1}.DIGESTS
    #    return 0
    #else
    #    rm ${1}.DIGESTS
    #    return 1
    #fi
}

As you may have guessed $1 is stage3-<arch>-<release>.tar.bz2.

gniourf_gniourf
  • 44,650
  • 9
  • 93
  • 104
user1935526
  • 31
  • 1
  • 2
  • 2
    Your question is not clear – perreal Dec 28 '12 at 22:53
  • you can clarify the confusion by adding expected inputs, and correct format for your require output. If you're getting error messages, please include the actual message using the `{}` editing tool near the top-left of the input box (click `edit` at the bottom). Good luck. – shellter Dec 28 '12 at 23:08
  • 1
    Useless use of `cat` spotted! – gniourf_gniourf Dec 29 '12 at 10:46

3 Answers3

7

Improvements:

TestStage() {
    local url sum_web sum_loc
    url="distfiles.gentoo.org/releases/${1:7:5}/current-stage3/${1}"
    wget -q "$url.DIGESTS"
    { read; read -r sum_web; } < "$1.DIGESTS"
    sum_loc=$(openssl dgst -r -sha512 "$1")
    ####### time to return
    [[ "${sum_web:0:128}" = "${sum_loc:0:128}" ]]
    return
}
  • Use of lower case variable names.
  • No use of the deprecated function keyword.
  • Use of $(...) instead of backticks.
  • Use of bash builtins instead of sed to get the second line of file "$1.DIGESTS". This saves a process spawn and a subshell (and a useless use of cat).
  • return on its own will return the return code of the previous statement, here the test statement.
  • Declare all local variables at once.

If you don't care about the file $1.DIGESTS that will be saved, you can also do:

TestStage() {
    local url sum_web sum_loc
    url="distfiles.gentoo.org/releases/${1:7:5}/current-stage3/${1}"
    { read; read -r sum_web; } < <(wget -q -O- "$url.DIGESTS")
    sum_loc=$(openssl dgst -r -sha512 "$1")
    ####### time to return
    [[ "${sum_web:0:128}" = "${sum_loc:0:128}" ]]
    return
}

Now, "${1:7:5}" will, as I understand it, expand to the second field of stage3-<arch>-<release>.tar.bz2 (where fields are separated by hyphens). You could also do:

IFS=- read _ arch _ <<< "$1"

In this case, your function would be:

TestStage() {
    local arch url sum_web sum_loc
    IFS=- read _ arch _ <<< "$1"
    url="distfiles.gentoo.org/releases/$arch/current-stage3/${1}"
    { read; read -r sum_web; } < <(wget -q -O- "$url.DIGESTS")
    sum_loc=$(openssl dgst -r -sha512 "$1")
    ####### time to return
    [[ "${sum_web:0:128}" = "${sum_loc:0:128}" ]]
    return
}

Hope this helps.

Then, use as:

if TestStage "stage3-<arch>-<release>.tar.bz2"; then
    # return value is true, proceed accordingly
else
    # return value is false, proceed accordingly
fi
gniourf_gniourf
  • 44,650
  • 9
  • 93
  • 104
  • +1 overall, but isn't `return` redundant in this case (but I'll grant you, self-documenting) ;-? Good luck to all. – shellter Dec 29 '12 at 21:22
  • @shellter, sure it is redundant and useless, but maybe this will be inside another if statement or loop, I don't know, that's the way he asked the question (I thought). – gniourf_gniourf Dec 29 '12 at 21:39
  • "Self-documenting"? Almost the reverse - until reading this answer, I would have assumed that a bare return would return 0, not "the return value of the previous line". – scubbo Oct 30 '22 at 17:41
1

I am not sure whether you could return comparison result like the way you done. You will most probably run into syntax error.

But you can try following syntax:

myfunction()
{
    [ "test" = "test" ]
}

myfunction
echo $?    # here if you get 0 that means strings are equal otherwise 1

In case you do decide to follow this approach, make sure comparison statement is the last statement in your function.

Icarus3
  • 2,310
  • 14
  • 22
0

You can't return anything other than integer values. You can either go with Ashish's suggestion and retrieve an exit code from $? after the function call or you could evaluate your string comparison expression by surrounding it with if ; then and then just echo any value indicating that the two hashes are either identical or not.

Community
  • 1
  • 1
J. Katzwinkel
  • 1,923
  • 16
  • 22