0

I'm attempting to write a bash script to check multiple files for a string, then if the string is found to remove it. Here is what I have, which I thought would work, but is only partly. The entries in the files are being removed, but running this a second time, I'm not getting the "nothing to do..." message.

#!/bin/bash
files=(
        '/etc/rsyslog.conf'
        '/etc/rsyslog.d/remote.conf'
        '/etc/rsyslog.d/01-remote.conf'
)

tmpcheck="for f in ${files[*]}; do  cat $f | grep blah | wc -l; done"
#for f in ${files[*]}; do  cat $f | grep collector.acuity.com | wc -l; done

if [[ "$tmpcheck" != 1 ]];then
for f in "${files[@]}";do
                echo -e "Removing blah from $f"
                sed -i "/blah/d" "$f"
        done
                echo -e "Restarting rsyslog service"
                systemctl restart rsyslog.service
else
        echo -e "Nothing to do, blah has been removed from $f"
fi

Any help would greatly help.

1

The main issue in your code (the assignment to tmpcheck) has been pointed out in other answers. Here is a slightly different approach, assuming that printing out the names of the modified/unmodified files is not mandatory:

if grep -q -- blah "${files[@]}"
then
    sed -i -e '/blah/d' -- "${files[@]}"
    systemctl restart rsyslog.service
else
    printf '%s\n' 'Nothing to do'
fi

The main ideas here are:

  • directly use a command's (grep) exit status in if compound commands instead of storing its output and testing it later, unless necessary;
  • calling utilities in a loop is less efficient; better to call them with several files as arguments whenever possible.

Also, while these are not issues in the code you show:

  • adding the end-of-options marker (--) protects against uncommon file names;
  • printf is safer than echo, especially when printing the result of expansions.
| improve this answer | |
1

I think you want to check for blah inside the for loop. There is no need for the tmpcheck variable. Instead you can use $restartrsyslog to restart rsyslog only once:

files=(
    '/etc/rsyslog.conf'
    '/etc/rsyslog.d/remote.conf'
    '/etc/rsyslog.d/01-remote.conf'
)

restartrsyslog=

for f in "${files[@]}";do
    if grep -q blah "$f"; then
        echo -e "Removing blah from $f"
        sed -i "/blah/d" "$f"
        restartrsyslog=yes
    else
        echo -e "Nothing to do, blah has been removed from $f"
    fi
done

if [[ -n $restartrsyslog ]] ; then
    echo -e "Restarting rsyslog service"
    systemctl restart rsyslog.service
fi
| improve this answer | |
0

Your tmpcheck variable is being set to a command not the output of that command, it will never equal 1 so that expression will always evaluate to true. You should use the $( ... ) construct to substitute the output of commands. You also don't need to loop over the files but if you do you should not use ${files[*]} but use ${files[@]} instead. Additionally there is no need to cat a single file and pipe into grep as grep can read files, and there is no need to pipe grep into wc -l as grep has a -c option to count the results. Finally why are you checking that tmpcheck doesn't equal 1? What if it equals 2 or more?

Does this work for you?

#!/bin/bash
files=(
    '/etc/rsyslog.conf'
    '/etc/rsyslog.d/remote.conf'
    '/etc/rsyslog.d/01-remote.conf'
)

tmpcheck=$(cat "${files[@]}" | grep -c blah)

if (($tmpcheck>=1)); then
    for f in "${files[@]}";do
        echo -e "Removing blah from $f"
        sed -i "/blah/d" "$f"
    done
    echo -e "Restarting rsyslog service"
    systemctl restart rsyslog.service
else
    echo -e "Nothing to do, blah has been removed from $f"
fi
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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