version control before making any changes, so you can see how the code evolves (and you can undo any changes)

Just curious, what is your monitor/editor size, what do you see?
I have 1024x768 with font:DejaVu Sans Mono,size:11, and I see , I see 103 chars/columns by 33 lines
If I go to font-size 10 I can see your 120 char

If I run your code through perltidy (as downloaded, with either configuration below)

## perltidy -olq -csc -csci=10 -cscl="sub : BEGIN END" -otr -opr -ce +-nibc -i=4 -pt=0 "-nsak=*" ## perltidy -olq -csc -otr -opr -ce -nibc -i=4 -pt=0 "-nsak=*"

The line count increases from ~1401 to ~1716, ~305 lines added for readability :)

And perltidy warns about having two functions named sendmail -- one from eod_functions and one from your main script

And perltidy also helps you keep consistent indentation level( you have some inconsistent ones)

Another aide to readability is increasing skimmability ( skimmable code is the idea ), that is replace long if/else blocks with functions, for example replace

my ($NL,$SKIP) = ($STATIC{NL},$STATIC{SKIP}); if ($STATIC{CHAR_NL} eq "NL"){ $NL = "\n"; }elsif($STATIC{CHAR_NL} eq "CR"){ $NL = "\r"; }elsif($STATIC{CHAR_NL} eq "CRLF"){ $NL = "\r\l"; }elsif($STATIC{CHAR_NL} eq "HEXNL"){ ##still needs testing $NL = ""; } if ($STATIC{CMD_SKIP} eq "SPACE"){ $SKIP = "\032"; }elsif($STATIC{CMD_SKIP} eq "NL"){ $SKIP = $NL; }else{ $SKIP = "$STATIC{CMD_SKIP}$NL"; }
with
my( $NL, $SKIP ) = optionize_nl_skip( \%STATIC );

I put in optionize cause its close but not quite a good name to describe what the function does, beside return nl and skip

Why write it this way? Its easier to test , like this

use strict; use warnings; use Test::More qw/ no_plan /;; { my( $NL, $SKIP ) = optionize_nl_skip( { qw/ CHAR_NL NL CMD_SKIP SP +ACE / }); is($NL, "\n" ); is($SKIP , "\33" ); ## Test::More printable bug } sub optionize_nl_skip{ my( $static ) = @_; my ($NL,$SKIP) = ( $static->{NL}, $static->{SKIP} ); if ($static->{CHAR_NL} eq "NL"){ $NL = "\n"; }elsif($static->{CHAR_NL} eq "CR"){ $NL = "\r"; }elsif($static->{CHAR_NL} eq "CRLF"){ $NL = "\r\l"; }elsif($static->{CHAR_NL} eq "HEXNL"){ ##still needs testing $NL = ""; } if ($static->{CMD_SKIP} eq "SPACE"){ $SKIP = "\032"; }elsif($static->{CMD_SKIP} eq "NL"){ $SKIP = $NL; }else{ $SKIP = "$static->{CMD_SKIP}$NL"; } return( $NL, $SKIP ); }

Also consider renaming sendmail, for example, this is the description for one of them

## SUB SENDMAIL ### DOES: generates and sends e-mail to all recipie +nts with attached csv report ##################

Maybe call it mail_report() or send_report() or report_result( \%REPORT,\%CONFIG,$RESULT )

Yeah, report_result, cause it explains action and object (send_result_report)

Other things that need comments (explanation) are regular expressions, because this

$TEMP[0] =~ s/^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[ +0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)://;
doesn't explain what $TEMP is, or why it needs ...numberregex... removed .... but the whole block should be a function call, something like
{ my( $commands, $matches ) ; ( $_, $commands, $matches ) = sanitize_and_then_some_c +ommands_matches( $_, \@METACHARS,\@TRANSLATIONS ); $DATA{$IP}->{COMMANDS} = $commands; $DATA{$IP}->{MATCHES} = $matches; }

Other minor things , style related, is this  unless ( $SHELL == '2' || $SHELL == '1' ) it will work, warnings won't complain, but if you're using numeric comparison operator == you might as well save yourself some typing of '' :) see perlnumber

More on style is  if ($#ARGV < 0 ) { &help($0); exit 0;} is perlishly written as  if( ! @ARGV ){ help($0); exit 1; }

or  @ARGV or die help($0);

Speaking of exit, if you put your main program into a subroutine Main you can write  Main( @ARGV ); exit( 0 ); More on this idea (and modulinos ) see Re: No such file or directory error/No such file or directory error, see template at (tye)Re: Stupid question (and see one discussion of that template at Re^2: RFC: Creating unicursal stars

Other minor issues are 2 argument open instead of 3 argument open, see open

Sometimes you use &function() but sometimes you use function() -- 99% of the time you don't need the & at all, so think about your reason for using it, then use the same style everywhere (good idea is to prefer less typing :)

speaking of function names and documentation, sub taint is poorly named, as taint has a specific meaning in perl and it isn't escape_ctrl, which is a better name and more documenting

speaking of documentation, PARSETEMPDB doesn't describe the database format , maybe you have an example database somewhere which describes the format, so this should be mentioned

another style thing is how you're UPPERCASENAMING lots and lots and lots and lots and lots of variables, there are various style recommendations about that, one being perlstyle

$ALL_CAPS_HERE constants only (beware clashes with perl vars!) $Some_Caps_Here package-wide global/static $no_caps_here function scope my() or local() variables

once you make some of these changes it becomes easier to see what your program is doing, how it works, and what other changes might be recommended

version control before making any changes, so you can see how the code evolves (and you can undo any changes)

start a new directory for each block/change :) poor-mans version control


In reply to Re: RFC: beginner level script improvement (version control) by Anonymous Monk
in thread RFC: beginner level script improvement by georgecarlin

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



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.