3
\$\begingroup\$

What my script does is:

  1. Append data to a file on a continuous basis.
  2. If an error pattern is observed, then do something.

I need feedback on the way I am grepping for patterns in the continuously appended file. For grep, I am creating a pattern file first and then using it with -f option to grep. Can I do this better? Any other better way I can write my over all logic.

Here's my script

#!/bin/bash
# List of error patterns to look for. Add more patterns to this list
p1='pattern1'
p2='pattern2'
p3='pattern3'
p4='pattern4'

PATTERNFILE=~/patternfile
FILETOCHECK=~/filetocheck

captureFileToCheck()
{
   //appends data into filetocheck &

}
createPatternFile()
{

    for i in `seq 100` # hopefully 100 is all we will need
    do
        if [ ! "x$p{$i}" = "x" ]
        then
            echo $p{$i} >> $PATTERNFILE
        else
            break
        fi
    done
}

createPatternFile
captureFileToCheck

while [ 1 ]
do

    if grep -f $PATTERNFILE $FILETOCHECK
    then
        echo "something starting"

          #doSometing

        echo "something complete"

        break
    fi

done
\$\endgroup\$
1
  • \$\begingroup\$ Any specific issue you're having? Security? Efficiency? Accuracy? Complexity? \$\endgroup\$
    – l0b0
    Feb 21, 2012 at 8:09

2 Answers 2

1
\$\begingroup\$

Here is another go at your function

patterns() {
cat <<EOF
pattern1
pattern2
pattern3
pattern4
EOF
}

However, the $patternfile is not a necessity. I would go with just

while :
do    
  grep -f <(patterns) $filetocheck && {
    echo found;
    break
  }
done
\$\endgroup\$
2
  • \$\begingroup\$ This is a good way to do it. I am concerned about portability! \$\endgroup\$ Apr 21, 2012 at 5:36
  • \$\begingroup\$ Ignore my comment about portability. Arrays are ok since this is explicitly bash. \$\endgroup\$ Apr 21, 2012 at 15:17
3
\$\begingroup\$

Use more quotes, as the saying goes.

I find while true is more clear, but the idiomatic while : is also popular.

You might want to use an array for the patterns, then you can simply loop over it (for pattern in "${patterns[@]}") instead of using seq and the if statement.

Uppercase variables are usually used for exports; that is, stuff which is relevant outside your script.

You should avoid cluttering ~; use tmp_dir="$(mktemp -d)" instead.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.