0

I have a script that parses a command:

while read line; do
        # The third token is either IP or protocol name with '['
        token=`echo $line | awk '{print $3}'`
        last_char_idx=$((${#token}-1))
        last_char=${token:$last_char_idx:1}
        # Case 1: It is the protocol name
        if [[ "$last_char" = "[" ]]; then
                # This is a protocol. Therefore, port is token 4
                port=`echo $line | awk '{print $4}'`
                # Shave off the last character
                port=${port::-1}
        else
                # token is ip:port. Awk out the port
                port=`echo $token | awk -F: '{print $2}'`
        fi
        PORTS+=("$port")
done < <($COMMAND | egrep "^TCP open")

for p in "${PORTS[@]}"; do
        echo -n "$p, "
done

This prints out ports like:

80,443,8080,

The problem is that trailing slash ,

How can I get the last port to not have a trailing , in the output ?

Thanks

Improve this question
6
  • Doesn't answer your immediate question, but it would be much more efficient if you changed read line to read f1 f2 token f4 _ -- no more awk needed if you let read do the splitting, and then you can assign port from either $f4 or $f2depending on the test. Commented Dec 19, 2018 at 21:29
  • Thanks Charles. Can you help with the request though? Commented Dec 19, 2018 at 21:30
  • ...as another aside, $COMMAND | ... is quite buggy; see BashFAQ #50 describing why, and various better-behaved alternatives. You also might consider using lower-case names for your own variables -- all-caps names are used for variables meaningful to the shell and operating system's tools, as specified by POSIX @ pubs.opengroup.org/onlinepubs/9699919799/basedefs/… Commented Dec 19, 2018 at 21:34
  • How about echo -n "${array[0]}"; unset array[0]; for p in "${array[@]}"; do echo -n ", $p"; done? Commented Dec 19, 2018 at 21:42
  • 1
    @StefanHamcke, that might as well be printf '%s' "${array[0]}"; printf ',%s' "${array[@]:1}"; no need for any echos, explicit looping, nor changing the array. That said, it's a good approach, and if you don't mind I'll fold it into my answer. Commented Dec 19, 2018 at 21:43

3 Answers 3

Reset to default
3

${array[*]} uses the first character in IFS to join elements.

IFS=,
echo "${PORTS[*]}"

If you don't want to change IFS, you can instead use:

printf -v ports_str '%s,' "${PORTS[@]}"
echo "${ports_str%,}"

...or, simplified from a suggestion by Stefan Hamcke:

printf '%s' "${PORTS[0]}"; printf ',%s' "${PORTS[@]:1}"

...changing the echo to printf '%s' "${ports_str%,}" if you don't want a trailing newline after the last port. (echo -n is not recommended; see discussion in the APPLICATION USAGE of the POSIX spec for echo).

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

how about

$ echo "${ports[@]}" | tr ' ' ','
Improve this answer

Comments

0

Why not simply:

( for p in "${PORTS[@]}"; do
    echo -n "$p, "
  done ) | sed -e 's/,$//'
Improve this answer

12 Comments

That's fairly inefficient -- the parens create a subshell (so fork()ing off a new process), the pipeline requires another layer of subprocess creation, and then the use of sed means we're fork()ing yet another subprocess only to use the execve() syscall to replace it with an external program.
I am a bit puzzled by this comment. Was sub-millisecond efficiency something that was looked for? This solution is simple, even if it is probably not the most efficient. Other solutions are certainly also worthwhile.
Shell scripts have a reputation of being slow -- in some quarters being described as too slow even to be suitable for the traditional role of starting services at boot time. Part of why they're considered slow is that they often are orders of magnitude slower (literally, ~100x slower) than they need to be due to widespread use of extremely inefficient practices. Teaching good practices whether or not they're immediately relevant means more people will be using those practices, so more scripts will be efficiently written in practice.
Running this 100 times in a loop with output to /dev/null, I get 0.425s wall-clock time; for the two-printf formulation based on Stefan's comment on the question, it's 0.005s for all 100 runs.
Simplicity, and being easy to understand and maintain are also important aims. Not all scripts need to be optimized to be hyper-efficient. Gaining 0.001 second on an utility script might not always be worthwhile.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.