Skip to content
Advertisement

what’s wrong with my shell script? when use loop in a function,always have some exception

this is my script:

#!/bin/bash
#adding values in an array
function addarray {
    local sum=0
    local newarray
    newarray=(`echo "$@"`)
    for value in $newarray
    do
        sum=$[ $sum+$value ]
    done
    echo $sum
}
myarray=(1 2 3 4 5)
echo "the origin array is : ${myarray[*]}"
arg1=`echo ${myarray[*]}`
echo $arg1
result=`addarray arg1`
echo "the result is $result

here is errormessage

the origin array is : 1 2 3 4 5
1 2 3 4 5
test11: line 9: 1 2 3 4 5: syntax error in expression (error token is "2 3 4 5")
the result is

My linux version is centos 7.0,and when i use loop only in script not in function,there will not be erros when process

Advertisement

Answer

There are a number of sources of confusion here. First, whenever you use anything like echo somethingorother in backquotes, you’re doing something weird and pointless; it takes the string (somethingorother in my example), converts it to output (of the echo command), then the backquotes capture the output and convert it back to a string. Net result: the echo and the backquotes effectively cancel each other, and you get the original string back. Well, mostly; there are several weird things that might happen to mess things up. Just skip both echo and backquotes, and use the string directly. So, for example, this line:

arg1=`echo ${myarray[*]}`

could be replaced by

arg1=${myarray[*]}

And BTW the name arg1 doesn’t match what this variable contains or how it’s used (well, sort of). It contains all the elements of myarray, not just the first. It also appears you meant its value to be passed as all of the arguments to addarray (although something different is happening, as I’ll explain in a bit).

Second, you’re handling arrays wrong. Take that command I just mentioned; it takes all the elements of the array, and sticks them together (with spaces between) into a single string, and stores that string in arg1. Thus, myarray was the array ("1" "2" "3" "4" "5"), but arg1 is just the single string “1 2 3 4 5”.

There’s some similar confusion inside the addarray function. You store the function’s arguments as an array with this:

newarray=(`echo "$@"`)

(well, mostly — this actually takes the arguments, carefully passes each on to echo as an argument, which sticks them together with spaces between, then that output is split into words based on spaces and each word is stored as an element of newarray. If there were any spaces within arguments, those will now be split into separate array elements, which is usually a mistake. Worse, if any of the arguments contain shell wildcards, they’ll be replaced by a list of matching files. This isn’t a problem here, but sometime you’re going to try to pass “*” to a function to tell it to multiply, and it’s going to wind up with a list of filenames and be very confused. The correct way to do this is newarray=("$@").)

Then you refer to newarray as if it was just a plain string variable here:

for value in $newarray

This takes just the first element of newarray (you’d need ${newarray[@]} or ${newarray[*]} to get all the elements). Then since it’s not double-quotes it splits that into words based on spaces, and then expands any wildcards into lists of matching filenames. When getting all elements of an array, you almost always want to use "${newarray[@]}" to get each element treated separately, but not word-split or wildcard-expanded.

Now the biggest problem. This line:

result=`addarray arg1`

runs the function addarray, and passes it the argument “arg1”. Not the variable arg1, not its value (“1 2 3 4 5”), just the text string “arg1”. Thus, inside the function, newarray gets just a single element, “arg1”; and the for loop runs once, with value set to “arg1”. That means the line inside the loop:

sum=$[ $sum+$value ]

expands to:

sum=$[ 0+arg1 ]

but since the inside of $[ ] is an arithmetic context, the string “arg1” is implicitly treated as a variable name and expanded to its contents, giving:

sum=$[ 0+1 2 3 4 5 ]

in which the “2 3 4 5” part makes no sense, giving the error you see.

Now let me add several stylistic notes: when declaring a function, funcname() is more standard and portable than function funcname. Use $(( )) instead of its obsolete equivalent, $[ ]. Use $( ) instead of backquotes to capture the output of a command — it’s easier to read, and backquotes have some weird syntactic consequences that can trip you up. Finally, it’s best practice to put variable references in double-quotes (e.g. echo "$sum" instead of echo $sum), even when it’s not strictly necessary.

Here’s a corrected version:

#!/bin/bash
#adding values in an array
addarray() {
    local sum=0
    local newarray
    newarray=("$@")    # This keeps each argument a separate array element
    for value in "${newarray[@]}"    # This iterates over the elements cleanly
    do
        sum=$(( sum+value ))    # In arithmetic context, $ is not needed for variables
    done
    echo "$sum"
}
myarray=(1 2 3 4 5)
echo "the origin array is : ${myarray[*]}"    # [*] mashes the elements together with spaces between, but that's ok here
result=$(addarray "${myarray[@]}")    # Pass each array element as a separate argument
echo "the result is $result"

Oh, and one more recommendation: shellcheck.net is great at pointing out common mistakes and bad practices in your scripts. While you’re learning, run your scripts through it, and it’ll save you from developing a variety of bad habits.

Advertisement