On Nov 16, 2007 2:16 AM, John W. Krahn <[EMAIL PROTECTED]> wrote:

> >
> > Any criticism or suggestion are welcome.
>
> OK, you asked for it.  From the monagt program:
>

Yes, thanks for pointing any problems out John.

>
> First, what is 'nc'?  I have an nc program on my HD but I don't think
>
> Somewhere else in the program you say it is used for port scanning.
> Perhaps if you used a more widely know program like nmap?

nc means 'netcat', which is shipped on redhat linux (from 2.6 kernel I
think) by default.
nc is lighter than nmap, which handle ports scanning slowly.
from `man nc' document:

NAME
     nc - arbitrary TCP and UDP connections and listens

DESCRIPTION
     The nc (or netcat) utility is used for just about anything under
the sun involving TCP or UDP.  It can open TCP con-nections, send UDP
packets, listen on arbitrary TCP and UDP ports, do port scanning, and
source routing.  Unlike telnet(1), nc scripts nicely, and separates
error messages onto standard error instead of sending them to standard
output, as telnet(1) does with some.



>
> Second, CAT?  Are you trying to win the UUOC award?
>

what's UUOC? cat is just cat under linux/unix, `man cat' pls.



> And third, it may be better to use the Net::Ping module instead of the
> external ping program.
>

Yes that's right. this function was not done by me, angel maybe
consider that to use less additional modules from cpan as possible as
she can. As you see,these two scripts don't call any external modules
from CPAN. But,I noticed that Net::Ping is a built-in module within
perl.


>
>       70     open (PIDFILE,$pid_file) or die "[EMERG] $!\n";
>       79 open (HDW,">",$pid_file) or die "[EMERG] $!\n";
>      201     open (STDIN, "</dev/null");
>      202     open (STDOUT, ">/dev/null");
>      203     open (STDERR,">&STDOUT");
>      215     open (HDW,">>",$log_file);
>      225     open (HDW,">>",$err_log);
>      234     open (HDW,">>",$err_log);
>
> Sometimes you use three arguments, sometimes two.  Sometimes you check
> the return value, sometimes not.
>

b/c the scripts were written and modified by two ppl, maybe she and me
have the different coding style. Anyway,thanks for pointing out this.



>
> You are matching the string 'inet addr:' SIX times when you need only
> match it twice.  Your patterns are not anchored so you could get false
> negatives, for example: the address 64.37.172.85 is a valid internet
> address yet the pattern /172\./ would exclude it (/10\./ and
> /192\.168\./ have the same problem.)  AFAIK all 127/8 addresses are
> local, not just 127.0.0/24.  According to RFC 1918:
>
> <quote>
> 3. Private Address Space
>
>    The Internet Assigned Numbers Authority (IANA) has reserved the
>    following three blocks of the IP address space for private internets:
>
>      10.0.0.0        -   10.255.255.255  (10/8 prefix)
>      172.16.0.0      -   172.31.255.255  (172.16/12 prefix)
>      192.168.0.0     -   192.168.255.255 (192.168/16 prefix)
>
>    We will refer to the first block as "24-bit block", the second as
>    "20-bit block", and to the third as "16-bit" block. Note that (in
>    pre-CIDR notation) the first block is nothing but a single class A
>    network number, while the second block is a set of 16 contiguous
>    class B network numbers, and third block is a set of 256 contiguous
>    class C network numbers.
> </quote>
>
> so your /172\./ pattern (even if it were anchored correctly) would
> exclude some valid internet addresses.
>

oops, yes, here the code for catching IPs have bugs.
I will upgrade it.


>
> You use /proc/meminfo in one place and free in the other but all of the
> same information is available from both so why two different methods?
>

yes both info can be got from the same location /proc/meminfo.
ok will upgrade it.

>
>      239 sub kill_children
>      240 {
>      241     kill TERM => keys %status;
>      242     sleep while %status;
>      243 }
>
> You are not modifying %status in the sub so will this ever return?
>

Yes the code above works pretty well.
b/c at the script's top we have redefined the handler for handling sigchld:

$SIG{CHLD}=sub {while((my $child=waitpid(-1,WNOHANG))>0){delete
$status{$child}}};

when killing TERM to childs, child exit and the handler above will
modify the %status.

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/


Reply via email to