Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

Re: How many bugs can *you* find

by merlyn (Sage)
on May 01, 2001 at 03:10 UTC ( #76807=note: print w/ replies, xml ) Need Help??


in reply to How many bugs can *you* find

OK, in five minutes, trying not to remember what anyone else said:

sub updateTiles { my $fto = $htmDir . 'tile.htm'; ## global var used my $content = ''; open(HOME,$fto); ## no checking for return value, could have redirect or pipe opens while (<HOME>) { $content .= $_ } ## inefficient close(HOME); my $paramTemp,$contentTemp; ## $contentTemp is NOT BEING DECLARED LOCAL (very misleading) my @sections = qw(Tile Pile Link); foreach $section (@sections) { ## no declaration of $section $contentTemp = $query->param($section); ## use of global $query. Why is contentTemp not declared here? if ($section eq 'Pile') { $contentTemp =~ s/[\n\r]/<p> +/g; } if ($section eq 'Link') { $contentTemp = "<img src=\"i +mages/enter.gif\" width=8 height=12><a href=\"cgi-bin/show.cgi?action +=showTiles&tileType=Search&searchFor=$contentTemp\">View this month's + tiles.</a>" } ## ampersands not entitized, inserted content not entitized or escape +d $content =~ s/<!--$section-->(.*)/<!--$section-->$cont +entTemp/; ## parens not needed on .*, what if $section has regex chars? } open(HOME,">$fto"); ## no checking return values; what if $fto starts with >? print HOME $content; ## could get IO error. What if visitor hits page while partially writ +ten? close(HOME); ## could get IO error. my $image = $query->param('Image'); if ($image ne '') { my $newFile = fileUpload('Image',250000,1, +'latest_image','JPEG','.jpg','.jpeg') } ## image might be undef if param not provided. }
This code is clearly not -w or strict compatible.

See what you can get for $10 of my time? How many of those would you have found for $10 of your time? {grin}

-- Randal L. Schwartz, Perl hacker


Comment on Re: How many bugs can *you* find
Download Code
Replies are listed 'Best First'.
(Ovid) Re(2): How many bugs can *you* find
by Ovid (Cardinal) on May 01, 2001 at 03:25 UTC

    New business idea: post code samples and try to get merlyn to offer free code reviews. Sell code reviews to clients...

    merlyn wrote:

    See what you can get for $10 of my time? How many of those would you have found for $10 of your time? {grin}

    Rather than list what I found, I'll list what I didn't. Then everyone can see what a faker I am :)

    my $paramTemp,$contentTemp; ## $contentTemp is NOT BEING DECLARED LOCAL (very misleading)

    Gah! I didn't see that one. (FYI: if you don't see it, "my" binds tighter than the comma).

    $content =~ s/<!--$section-->(.*)/<!--$section-->$contentTemp/; ## parens not needed on .*, what if $section has regex chars?

    Saw the useless parens (and that despicable dot star!), but didn't think about $section having regex characters.

    if ($image ne '') { my $newFile = fileUpload('Image',250000,1,'lat +est_image','JPEG','.jpg',' +.jpeg') } ## image might be undef if param not provided.

    Sigh. I missed this one, too.

    All in all, I don't feel bad about catching most of the errors. This little post was 17 lines of code. Imagine expanding this out to over 2,000!

    Cheers,
    Ovid

    Update: It's interesting to notice that someone can hack together a script that has virtually every line of code in error and have the script still work. The person who wrote this code was a coder, not a programmer.

    Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

      Said merlyn, and then Ovid:
      $content =~ s/<!--$section-->(.*)/<!--$section-->$contentTemp/; ## parens not needed on .*, what if $section has regex chars?
      Saw the useless parens (and that despicable dot star!), but didn't think about $section having regex characters.
      Actually, $section can't have regex characters as the code is written, since it simply iterates over the hard-coded items in @sections.

      That will be five dollars, please. I take paypal, and I don't take American Express.

         MeowChow                                   
                     s aamecha.s a..a\u$&owag.print
        Actually, $section can't have regex characters as the code is written, since it simply iterates over the hard-coded items in @sections
        As written, but not as maintained. {grin}

        For this to pass code review for me, there'd either need to be a fix to that line to make it work even when $section has regex chars, or a note put up above saying "don't ever include regex chars in this string... see below".

        The reason I flag that is that it creates an intertwining dependency that will be hell for the maintenance programmer down the road. Trust me: I've been that maintenance programmer enough times to know how much I hate those things. That's why my code reviews are so thorough. {grin}

        -- Randal L. Schwartz, Perl hacker

Re: Re: How many bugs can *you* find
by buckaduck (Chaplain) on May 01, 2001 at 19:49 UTC
    Wow. By my calculations, that's $120 an hour. And yet, I'm tempted to scrape up some money, gather a bunch of my scripts together, and see what Randal can do with them in an hour...

    buckaduck

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://76807]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others studying the Monastery: (10)
As of 2015-07-08 03:51 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The top three priorities of my open tasks are (in descending order of likelihood to be worked on) ...









    Results (94 votes), past polls