http://qs321.pair.com?node_id=125498


in reply to Looking for feed back on a guestbook

Your script has a race condition. That is, two copies of your script can be running at the same time and they both can try to write a new entry to the guestbook, resulting in it being curropted. To fix this problem you can use flock. (Just use Fcntl qw(:flock); ... flock HTML, LOCK_EX; would probably be enough, see the docs for details.)

A more minor issue with your script is that print start_html() will print out <html> and a <head> section for your document. You probably shouldn't use it, unless you plan to use CGI to generate all your HTML. (Especially considering that CGI will output XHTML in newer versions, making it so browsers would be welcome to reject your apparently well-formed HTML.)

I notice that you're testing variables for a pattern and then preforming subtitutions for that pattern. It would be clearer, and probably just as fast, to just try the subtitution -- if the pattern's not found, nothing will happen. Personally, I would write your repetitive substitutions of <, etc. for the appropriate entities with a foreach loop:

foreach ($name,$mail,$message) { s/</&lt;/g; s/>/&gt;/g; }

A final potential problem in your script is that you might want to think of a better way of handling situtations where certain input fields are blank.

In your use of CGI.pm you are mixing the object-oriented ($q->something) and function-based (something()) interfaces. It would be clearer if you stuck with only one.

update: I notice you use #!/use/bin/perl -w. I suspect you meant #!/usr/bin/perl -w

Replies are listed 'Best First'.
Re: Re: Looking for feed back on a guestbook
by Cobo (Scribe) on Nov 15, 2001 at 08:02 UTC
    Ok, thank you very much. and about the typo, can you tell I'm running windows at the moment? lol ( and if any one is interested (which they aren't) I have a duel boot, I'm running windows at the moment because i can't get any multi-messengers to work and told somebody I'd talk to them on msn)