3

What's wrong with the following? I get a [: too many arguments error.

if [ 'wc -l pid.txt | awk '{print $1}'' -ge "1" ]
then
    for line in $(cat pid.txt)
    do
        kill $line
    done
else
    rm pid.txt
fi

closed as off-topic by ilkkachu, Rui F Ribeiro, slm Aug 4 '18 at 0:45

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions describing a problem that can't be reproduced and seemingly went away on its own (or went away when a typo was fixed) are off-topic as they are unlikely to help future readers." – ilkkachu, Rui F Ribeiro, slm
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    The single quotes around your command should be back-ticks. Is that a typo in your post or are you using the wrong symbol? – Andy Dalton Aug 3 '18 at 20:30
  • oops I feel dumb. I am using single quotes and thats the problem. Thanks a lot. – Vas Aug 3 '18 at 20:36
  • 1
    @Vas Your "problem" was the single quotes, but there's a lot more that could be improved. – Kusalananda Aug 3 '18 at 20:58
5

Improved further thanks to Kusalananda.

  • Testing for contents of the file is not necessary as the while read loop won't run on an empty file.
  • You could then simply use [ -s pid.txt ] || rm pid.txt to remove the pid file. However, do you really have a reason to keep the file after processing? It seems likely you would want to remove it regardless.

while read -r line
do
    kill "$line"
done < pid.txt
[ -s pid.txt ] || rm pid.txt

A while read loop is being used instead of for $(cat ...). Don't read lines with for.

The -r option specifies "do not allow backslashes to escape any characters" which shouldn't really be an issue in a file of pids, but it is best practice to always set unless there is a specific reason not to.

  • 3
    The initial test can be removed completely as the read loop won't run if it's an empty file. The rm could be replaced by [ -s pid.txt ] && rm pid.txt to remove the file if it's empty. The inner loop could be replaced by xargs -r kill <pid.txt. – Kusalananda Aug 3 '18 at 20:52
  • One could also grep . pid.txt | while read ... if there's empty lines in the pid.txt file that one wants to avoid. – Kusalananda Aug 3 '18 at 21:09
  • 1
    [ -s pid.txt ] || rm pid.txt to remove if empty, right? – ilkkachu Aug 3 '18 at 23:48
  • @ilkkachu Sorry, got the test backwards... duh. – Kusalananda Aug 4 '18 at 7:26

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