0

I write this script:

for ip in ${ARRAY[*]}; do
  for e in ${ARRAY[*]}; do
    echo $e
  done | grep -c "$ip" | if [[ "$(cat)" -lt "10" && $sent != *"$ip"* ]]; then
      sent=$sent$ip
      sed -n "/$ip/p" $1 | mail -s "subject" "mail@mail.mail"
    fi
done

I want to get each element from ARRAY and count how many time This item is in ARRAY. If this count is less than 10 and this element is the first time it is counted (to avoid count an element twice), an email is sent.

However, sent variable does not save the strings concatenate. If I made an echo $ sent just after its assignment, sent returns the expected result.

If I make echo $ sent just after fi, the variable is already empty. Obviously, by this way, two elements are tacking in account and two mails are sent instead of just one.

What am I doing wrong?

Biffen
  • 6,249
  • 6
  • 28
  • 36
Miguel.G
  • 377
  • 1
  • 6
  • 20
  • 1
    Piping (`|`) creates subshells, whose variables aren’t ‘visible’ in the ‘parent’ shell. – Biffen Mar 23 '18 at 10:51
  • 1
    Possible duplicate of [bash piping prevents global variable assignment](https://stackoverflow.com/questions/6255303/bash-piping-prevents-global-variable-assignment) – Biffen Mar 23 '18 at 10:53
  • The duplicate is technically the immediate solution to the question in the title, but there is a lot which is broken here, so I think answering makes more sense than duplicating. – tripleee Mar 23 '18 at 11:03

2 Answers2

1

Your attempt has a number of issues in the syntax as well as efficiency. Try http://shellcheck.net/ for the immediate fixes (incorrect quoting, etc) but you really should refactor to something like

printf '%s\n' "${ARRAY[@]}" |
sort | uniq -c | sort -rn |
while read -r count ip; do
    case $count in [1-9]) break;; esac   # stop looping after 10
    sed -n "/$ip/p" "$1" | mail -s "subject" "mail@mail.mail"
done

The overhead of sorting should be negligible for anything which makes sense to store in a Bash array in the first place (though putting this in an array may well be your fundamental error, actually) and simplifies the rest of the script significantly. This way, you avoid having to keep track of duplicates in the downstream code, too, and you avoid looping over the same values and files multiple times.

tripleee
  • 175,061
  • 34
  • 275
  • 318
0

This is the same as "Bash variable scope": you're piping into the if statement, which creates a sub-shell. The sub-shell has a new scope, which is lost after the if statement is done.

Among many solutions, you could for example echo every update of sent and tail -n1 the whole statement in the end to only show the last version of sent:

for ip in "${ARRAY[@]}"; do
  for e in "${ARRAY[@]}"; do
    echo "$e"
  done | grep -c "$ip" | if [[ "$(cat)" -lt "10" && $sent != *"$ip"* ]]; then
      sent=$sent$ip
      sed -n "/$ip/p" "$1" | mail -s "subject" "mail@mail.mail"
      echo "$sent"
    fi
done | tail -n1
tripleee
  • 175,061
  • 34
  • 275
  • 318
Marcel Steinbach
  • 1,109
  • 6
  • 9