|
I think your script is not bad at all (especially since you say it works!) but here are some opinionated style points that may be of interest: I would not personally check for the existence of a file before deleting it unless I wanted to do something differently with the existing file like archive or append. "rm filename" fails (exit status non-zero) when filename does not exist. "rm -f filename" returns zero even if the file exists. A bonus is you can save almost 20 lines total: "rm -f /tmp/topips /tmp/topqueues". The reason you might check first is because you don't want stderr to poo poo up your cron logs or something. You might or might not already be aware of the special variables such as "$?". You can trial in the interactive shell with "echo $?". You append to a file which you deleted, which causes it to exist even if zero bytes are written, so you don't need to check for it later. (try this at home: "echo -n >> loltestfile ; xxd loltestfile") I don't use SASL on my mail server so I don't have logs to test with but maybe you could eliminate the use of awk in some places by using sort -k ? Also as far as I can tell you set IP= for no reason. I don't know how fail2ban works but maybe you could just tell fail2ban about your count of emails instead of doing the firewall rule yourself (not that there's anything wrong with you doing so but fail2ban already solves a similar problem which is one IP fails too many auths within some time period) the benefit being that fail2ban will unblock after some time.
|
#
¿
Oct 17, 2014 01:39
|
|
|
|
| # ¿ Apr 29, 2024 05:45 |
|
|
I think this is a good iteration. I think I was not very clear when I mentioned $?. I was referring to rm vs rm -f (but as you discovered it works everywhere!) If the file does not exist rm alone will "fail" and $? is 1 and it will emit something on stderr that may end up in a log file somewhere, but rm -f does not care whether it exists or not and $? is always zero and emits no error (so logs do not get filled with garbage). You can also $? with grep as you are doing, though if you are only checking success/failure of the match and don't care about capturing the matched line, you can also invoke grep with -q and put the whole expression in the if block e.g: code:I learned a lot about bash by second-guessing myself and finding a highly upvoted stack exchange post or obscure TLDP reference about it. I completely agree with your comment about bash's capacity to drive one to drink. I think it's power can be deceptive -- I learned a lot more about the interactive shell when learning about scripting, and vice-versa, but I still don't always know when trying to map a particular problem into bash if I should call it quits and switch to a more feature-ful language. One last thing I thought of, some potential for trouble exists if the previous instance of your script has not finished processing before the next one starts. Although when that happens it probably means something else is wrong (processing more than a minute's worth of IPs/maillogs!), you may want to read about mktemp, which can help ensure the temporary files in (accidentally) simultaneous instances don't collide (and may actually be interesting if you are enjoying bash so far...!). When you start worrying about this stuff life can become more complicated. Handling those situations gracefully in a way that works on all shell variants is challenging but it's probably doable for a single shell (e.g. bash) and might even be fun to OCD about when you've got nothing else to keep you busy =)
|
|
#
¿
Oct 20, 2014 02:58
|
|