Beefy Boxes and Bandwidth Generously Provided by pair Networks
XP is just a number
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??
Looks cool! A few notes:
  1. Most Perl programs that are meant to be run from the shell use Getopt::Long and Pod::Usage for interfacing with the user. You might want to look at those as they can really simplify your life. Plus, they allow you to easily handle options that are, well, optional. :-)
  2. You use a bunch of tempfiles. The way you're using it means that you cannot safely have two versions of this program running, plus it's not OS-portable the way you've written it. (In fact, it's barely Unix-portable.) I'd recommend using File::Temp which handles all that crap for you.
  3. Although you don't actually use directories and pathnames, I'd also look at File::Spec (and File::Spec::Functions if you don't like the OO interface) for that need.
  4. You use 'rm -rf' instead of the built-in unlink. Why? (Plus, if you use File::Temp, it's taken care of for you.)
  5. It would probably be better to write
    for($i=$addr;$i<$saddr;$i++) { print TEMP "\n" }
    as
    print TEMP $/ for $addr .. $saddr - 1;
    Not only is it more Perlish, but it better documents what it is you're attempting to do.
  6. You don't check the return value for any of your system calls. That's not good ...
  7. Are you charged for your whitespace?
    $x=~s/...//g;
    is hard to read. Why not do something like
    $x =~ s/...//g;
    instead? The compiler doesn't care, but I will. Reading code is hard - you probably want to make it as easy as possible. (Plus, see below for more help in readability.)
  8. Your main loop
    open(ITEMP,"/tmp/scresult.tmp"); @data=<ITEMP>; close ITEMP; foreach $x(@data) { if($x=~/^\(gdb\)/) { $x=~s/\(gdb\) 0x.+ <.+>:\s+//g; $x=~s/0x/\\x/g; $x=~s/\n//g; $x=~s/\(gdb\)//g; $x=~s/\s+//g; $sc.=$x; } $i++; # # "Indent" the shellcode # ($i exists only for this) # if(($i%12)==0) { $sc.="\"\n\t\"" } }
    is probably better written as:
    open( TEMP, "</tmp/scresult.tmp" ) or die "Cannot open: $!\"; my $sc="char ".$func."[]=\n\t\""; my $i = -1; while (<TEMP>) { if(/^\(gdb\)/) { s/\(gdb\) 0x.+ <.+>:\s+//g; s/0x/\\x/g; s/\n//g; s/\(gdb\)//g; s/\s+//g; $sc .= $_; } # "Indent" the shellcode # ($i exists only for this) $i++; unless (++$i%12) { $sc.="\"\n\t\"" } } close TEMP;

    Not only is it clearer what you're doing, but you don't slurp the whole file into memory. What if you're going for 1G of data? Also, you should indent your comments to line up with your code. Otherwise, the whole point of indenting - to show logical groupings - is ruined. When I skimmed your original code, I didn't immediately see the fact that the modulo-check on $i had something to do with the loop.

    Plus, you shouldn't declare a variable until you intend to use it. You declare $sc way up at the top, but don't use it until nearly the end. If you declare it at the top, I'm going to look for usage near the top. If I don't see it, I'm going to start looking for bugs that may not be there.

Being right, does not endow the right to be rude; politeness costs nothing.
Being unknowing, is not the same as being stupid.
Expressing a contrary opinion, whether to the individual or the group, is more often a sign of deeper thought than of cantankerous belligerence.
Do not mistake your goals as the only goals; your opinion as the only opinion; your confidence as correctness. Saying you know better is not the same as explaining you know better.


In reply to Re: mksc.pl by dragonchild
in thread mksc.pl by X-3mE

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others examining the Monastery: (7)
As of 2024-03-28 11:14 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found