# How to improve the way I handle greping in this script

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  - Any specific issue you're having? Security? Efficiency? Accuracy? Complexity? – l0b0 Feb 21 '12 at 8:09 add comment ## 2 Answers 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  - This is a good way to do it. I am concerned about portability! – abc Apr 21 '12 at 5:36 Ignore my comment about portability. Arrays are ok since this is explicitly bash. – rahul Apr 21 '12 at 15:17 add comment 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.

-