CODEWITHSUNDEEP
linuxbashshellscriptingcounter
Sparsh Gupta
Sparsh Gupta

Reputation: 2233

Counter increment in Bash loop not working

I have the following simple script where I am running a loop and want to maintain a COUNTER. I am unable to figure out why the counter is not updating. Is it due to subshell that's getting created? How can I potentially fix this?

#!/bin/bash

WFY_PATH=/var/log/nginx
WFY_FILE=error.log
COUNTER=0
grep 'GET /log_' $WFY_PATH/$WFY_FILE | grep 'upstream timed out' | awk -F ', ' '{print $2,$4,$0}' | awk '{print "http://domain.example"$5"&ip="$2"&date="$7"&time="$8"&end=1"}' | awk -F '&end=1' '{print $1"&end=1"}' |
(
while read WFY_URL
do
    echo $WFY_URL #Some more action
    COUNTER=$((COUNTER+1))
done
)

echo $COUNTER # output = 0

Upvotes: 170

Views: 436694

Answers (13)

Znik
Znik

Reputation: 1136

Source script has some problem with subshell. First example, you probably do not need subshell. But We don't know what is hidden under "Some more action". The most popular answer has hidden bug, that will increase I/O, and won't work with subshell, because it restores couter inside loop.

Do not fortot add '\' sign, it will inform bash interpreter about line continuation. I hope it will help you or anybody. But in my opinion this script should be fully converted to AWK script, or else rewritten to python using regexp, or perl, but perl popularity over years is degraded. Better do it with python.

Corrected Version without subshell:

#!/bin/bash
WFY_PATH=/var/log/nginx
WFY_FILE=error.log
COUNTER=0
grep 'GET /log_' $WFY_PATH/$WFY_FILE | grep 'upstream timed out' |\
awk -F ', ' '{print $2,$4,$0}' |\
awk '{print "http://example.com"$5"&ip="$2"&date="$7"&time="$8"&end=1"}' |\
awk -F '&end=1' '{print $1"&end=1"}' |\
#(  #unneeded bracket
while read WFY_URL
do
    echo $WFY_URL #Some more action
    COUNTER=$((COUNTER+1))
done
# ) unneeded bracket

echo $COUNTER # output = 0

Version with subshell if it is really needed

#!/bin/bash

TEMPFILE=/tmp/$$.tmp  #I've got it from the most popular answer
WFY_PATH=/var/log/nginx
WFY_FILE=error.log
COUNTER=0
grep 'GET /log_' $WFY_PATH/$WFY_FILE | grep 'upstream timed out' |\
awk -F ', ' '{print $2,$4,$0}' |\
awk '{print "http://example.com"$5"&ip="$2"&date="$7"&time="$8"&end=1"}' |\
awk -F '&end=1' '{print $1"&end=1"}' |\
(
while read WFY_URL
do
    echo $WFY_URL #Some more action
    COUNTER=$((COUNTER+1))
done
echo $COUNTER > $TEMPFILE  #store counter only once, do it after loop, you will save I/O
)

COUNTER=$(cat $TEMPFILE)  #restore counter
unlink $TEMPFILE
echo $COUNTER # output = 0

Upvotes: 1

Augustus Hill
Augustus Hill

Reputation: 71

There were two conditions that caused the expression ((var++)) to fail for me:

  1. If I set bash to strict mode (set -euo pipefail) and if I start my increment at zero (0).

  2. Starting at one (1) is fine but zero causes the increment to return "1" when evaluating "++" which is a non-zero return code failure in strict mode.

I can either use ((var+=1)) or var=$((var+1)) to escape this behavior

Upvotes: 7

IgorAlves
IgorAlves

Reputation: 5550

This is a simple example 

COUNTER=1
for i in {1..5}
do   
   echo $COUNTER;
   //echo "Welcome $i times"
   ((COUNTER++));    
done

Upvotes: 2

Dennis Williamson
Dennis Williamson

Reputation: 360095

Instead of using a temporary file, you can avoid creating a subshell around the while loop by using process substitution.

while ...
do
   ...
done < <(grep ...)

By the way, you should be able to transform all that grep, grep, awk, awk, awk into a single awk.

Starting with Bash 4.2, there is a lastpipe option that

runs the last command of a pipeline in the current shell context. The lastpipe option has no effect if job control is enabled.

bash -c 'echo foo | while read -r s; do c=3; done; echo "$c"'

bash -c 'shopt -s lastpipe; echo foo | while read -r s; do c=3; done; echo "$c"'
3

Upvotes: 13

C.E. Montijo
C.E. Montijo

Reputation: 39

This is all you need to do:

$((COUNTER++))

Here's an excerpt from Learning the bash Shell, 3rd Edition, pp. 147, 148:

bash arithmetic expressions are equivalent to their counterparts in the Java and C languages.[9] Precedence and associativity are the same as in C. Table 6-2 shows the arithmetic operators that are supported. Although some of these are (or contain) special characters, there is no need to backslash-escape them, because they are within the $((...)) syntax.

..........................

The ++ and - operators are useful when you want to increment or decrement a value by one.[11] They work the same as in Java and C, e.g., value++ increments value by 1. This is called post-increment; there is also a pre-increment: ++value. The difference becomes evident with an example:

$ i=0
$ echo $i
0
$ echo $((i++))
0
$ echo $i
1
$ echo $((++i))
2
$ echo $i
2

See http://www.safaribooksonline.com/a/learning-the-bash/7572399/

Upvotes: 3

pkm
pkm

Reputation: 2783

count=0   
base=1
(( count += base ))

Upvotes: 11

geekzspot
geekzspot

Reputation: 875

minimalist

counter=0
((counter++))
echo $counter

Upvotes: 8

Bill Parker
Bill Parker

Reputation: 781

COUNTER=$((COUNTER+1))

is quite a clumsy construct in modern programming.

(( COUNTER++ ))

looks more "modern". You can also use 

let COUNTER++

if you think that improves readability. Sometimes, Bash gives too many ways of doing things - Perl philosophy I suppose - when perhaps the Python "there is only one right way to do it" might be more appropriate. That's a debatable statement if ever there was one! Anyway, I would suggest the aim (in this case) is not just to increment a variable but (general rule) to also write code that someone else can understand and support. Conformity goes a long way to achieving that.

HTH

Upvotes: 59

Jay Stan
Jay Stan

Reputation: 1217

COUNTER=1
while [ Your != "done" ]
do
     echo " $COUNTER "
     COUNTER=$[$COUNTER +1]
done

TESTED BASH: Centos, SuSE, RH

Upvotes: 112

glenn jackman
glenn jackman

Reputation: 246807

I think this single awk call is equivalent to your grep|grep|awk|awk pipeline: please test it. Your last awk command appears to change nothing at all.

The problem with COUNTER is that the while loop is running in a subshell, so any changes to the variable vanish when the subshell exits. You need to access the value of COUNTER in that same subshell. Or take @DennisWilliamson's advice, use a process substitution, and avoid the subshell altogether.

awk '
  /GET \/log_/ && /upstream timed out/ {
    split($0, a, ", ")
    split(a[2] FS a[4] FS $0, b)
    print "http://example.com" b[5] "&ip=" b[2] "&date=" b[7] "&time=" b[8] "&end=1"
  }
' | {
    while read WFY_URL
    do
        echo $WFY_URL #Some more action
        (( COUNTER++ ))
    done
    echo $COUNTER
}

Upvotes: 12

bos
bos

Reputation: 6535

First, you are not increasing the counter. Changing COUNTER=$((COUNTER)) into COUNTER=$((COUNTER + 1)) or COUNTER=$[COUNTER + 1] will increase it.

Second, it's trickier to back-propagate subshell variables to the callee as you surmise. Variables in a subshell are not available outside the subshell. These are variables local to the child process.

One way to solve it is using a temp file for storing the intermediate value:

TEMPFILE=/tmp/$$.tmp
echo 0 > $TEMPFILE

# Loop goes here
  # Fetch the value and increase it
  COUNTER=$[$(cat $TEMPFILE) + 1]

  # Store the new value
  echo $COUNTER > $TEMPFILE

# Loop done, script done, delete the file
unlink $TEMPFILE

Upvotes: 187

yjshen
yjshen

Reputation: 6693

It seems that you didn't update the counter is the script, use counter++

Upvotes: 0

dbf
dbf

Reputation: 6499

Try to use 

COUNTER=$((COUNTER+1))

instead of

COUNTER=$((COUNTER))

Upvotes: 18

Related Questions