[Ilugc] Auto e-mail greeter program

  • From: girishvenkatachalam@xxxxxxxxx (Girish Venkatachalam)
  • Date: Mon Oct 16 15:29:43 2006

On Mon, Oct 16, 2006 at 09:41:04AM +0200, Binand Sethumadhavan wrote:

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?
Very sorry, I meant to remove it. Honestly!

My idea was to make it look good but later reverted back. It is not reqd at 
all. I will upload a new version.

Thanks for pointing out.

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.
Oh yes, I know mktemp but forgot. I could put that change in but I am not too 
particular. :-)

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.
There is a reason. :-)

I have clearly written that it is mnemnonic. 

That is actually meant to solve the confusion between mm/dd and dd/mm formats 
for dates. You can easily look at an entry and identify.

That field is meant for humans, not for the program.

4. All your lines which goes "| cut -d '|' ..." can be replaced far
cleaner with the bash inbuilt command "set".
How? Can you give a code sample? I guess I get what you are saying but some 
prodding will help. :-)

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,

        I should thank you a million times for this bcoz you really made me 
look a fool and I really appreciate it.

        Because I was actually looking for a construct like that and I did not 
know the "while read line" thing.

        I tried it and it works.

        Besides your argument of avoiding external commands, my argument for 
this would be that it is really clean and simple.

        It is like using 10 words to explain something that can be told in one 
word.

        Which is better?

        Thanks. I learnt something new today.

        ILUGC rocks ! :-)

regards,
Girish

Other related posts:

  1. Home
  2. ilugc
  3. Archive
  4. 10-2006