[Ilugc] Auto e-mail greeter program
- From: binand@xxxxxxxxx (Binand Sethumadhavan)
- Date: Mon Oct 16 13:11:14 2006
On 16/10/06, Girish Venkatachalam <girishvenkatachalam@xxxxxxxxx> wrote:
PS:- Comments, criticisms,flames, suggestions all welcome. Please
discuss on the list instead of sending me personal e-mail
A commendable effort. I have only four points to make.
1. From your setup script:
echo "Copying image files........."
sleep 1
\cp anniv.png ${HOME}/.aeg
That is the Microsoft way of doing things :) Why do you have that sleep there?
2. Your aeg script has a race condition; on multiuser systems with
malicious users, this can lead to arbitrary files owned by the user
running AEG to be wiped out. mktemp(1) is the safe way of creating
temporary files.
3. You don't seem to be using the fourth field of your events file
anywhere. In fact, it is identical to the fifth field.
4. All your lines which goes "| cut -d '|' ..." can be replaced far
cleaner with the bash inbuilt command "set".
The trick in writing good shell scripts is to use bash builtins more
and external commands less, for maximum efficiency. For example, your
loop:
while [ $lines -ne 0 ]; do line=`sed -n ${lines}p $file`; do_stuff
$line; lines=`expr $lines - 1`; done
can be rewritten as
while read line; do do_stuff $line; done < $file
and you have eliminated the external sed & expr program invocations
and also the need for your $lines variable.
Binand
Other related posts: