0

I have a function:

function checkConn()
{
    RET=0
    echo "in function: ${2}"
    echo
    for i in `cat ${1}`
    do
        for p in ${2}
        do
            if nc -dzw 2 ${i} ${p}  2>&1 >/dev/null

and so on.

In the "main" body of the script, I have the following:

PORTS='22 161 162 1521'
checkConn ${FILE} ${PORTS}

FILE is the name of a file which contains a list of IPs.

When I pass PORTS to the function, only the 1st item gets passed. I also tried it with double-quotes.

I put that "echo" statement to confirm. It only shows the first item in PORTS, which is 22.

How can I pass all the ports to this function, and then loop through each one?

4
  • Quote your variables Commented Apr 4, 2017 at 15:20
  • shellcheck.net is generally a good place to start, by the way -- you're missing a lot of quotes that are necessary to avoid being surprised by corner-case behavior when processing unusual data. See also Don't Read Lines With For Commented Apr 4, 2017 at 15:24
  • Also, don't use string variables to store lists. ports=( 22 161 162 1521 ) is the correct way to define an array named ports. checkConn "$file" "${ports[@]}" will pass the filename as the first argument and the individual ports as subsequent arguments; inside that function, one can do something like checkConn() { file=$1; shift; for port; do echo "Checking port "$port"; done; } -- iterating over the argument vector. Commented Apr 4, 2017 at 15:27
  • (And don't use all-caps variable names -- see pubs.opengroup.org/onlinepubs/9699919799/basedefs/…, defining all-caps names as for use for variables with meaning to the operating system or shell, and reserving names with lowercase characters for application use; since setting a shell variable overwrites any like-named environment variable, the naming conventions apply in both places). Commented Apr 4, 2017 at 15:29

2 Answers 2

2

Best practice is to pass the list of ports as individual arguments, each with their own argv entry -- like so:

checkConn() {
  file=$1; shift  ## read filename from $1, then remove it from the argument list

  while IFS= read -r address; do
    for port; do  ## we shifted off the filename so this only iterates over ports
      if nc -dzw 2 "$address" "$port" </dev/null >/dev/null 2>&1; then
        echo "$address $port OPEN"
      else
        echo "$address $port CLOSED"
      fi
    done
  done <"$file"
}

file=addrs.txt
ports=( 22 161 162 1521 )
checkConn "$file" "${ports[@]}"

Notes:

  • The function keyword is not actually required for use to define a function; it makes your code incompatible with POSIX sh, but provides no benefit over the portable syntax. Avoid it.
  • The while IFS= read -r idiom is described in detail in BashFAQ #1. Also see Don't Read Lines With For.
  • Arrays, as used in the ports=( ... ) and "${ports[@]}" syntax, are described in the BashGuide.
Sign up to request clarification or add additional context in comments.

Comments

1

Multiple syntax violations and outdated constructs, you probably need something like,

function checkConn() {
    # define variables as local unless you are using it beyond the scope
    # of the function and always lower-case variable names 
    local ret=0
    printf "%s\n" "$2"

    # Splitting the string into an array so that it can be accessed 
    # element wise inside the loop. The -a option in read stores the  
    # elements read to the array
    read -ra portList <<<"$2"     

    # Input-redirection of reading the file represented by argument $1
    # representing the file name. The files are read one at a time    
    while IFS= read -r line; do
        # Array values iterated in a for-loop; do the action
        # for each value in the port number
        for port in "${portList[@]}"; do
            if nc -dzw 2 "$line" "$port"  2>&1 >/dev/null; then
                printf "%s %s\n" "$line" "$port"
                # Your rest of the code
            fi
        done
    done < "$1"
}

and call the function as

ports='22 161 162 1521'
filename="file"
checkConn "$filename" "$PORTS"

2 Comments

Your for loop is going to run exactly once, with port set to the value of portList.
It's a good answer. We differ only on some fairly minor points. :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.