Re: a little help...
- From: japhy@xxxxxxxxxxxx (Jeff 'japhy' Pinyan)
- Date: Wed, 28 Sep 2005 15:24:41 -0400 (EDT)
On Sep 28, FamiLink Admin said:
I am trying to read a log file and get a list of how many times an IP address get blocked each hour by category PO. An example line in the log with a block is:
-------------
[2005-09-28 10:05:03 -7:00] 127.0.0.1 71.32.59.249 216.163.137.3 - http://www.playboy.com/ blocked 0 PO
-------------
What I have kinda works but I am not sure if it is the best practice. This is the first time programming in perl and this is what I have so far:
Your indentation leaves much to be desired, so I've "fixed" it.
sub Scanlog {
local($ipb) = @_;
No reason to use 'local'; stick with 'my' here. But... what is $ipb? You don't use it anywhere!
open my $slog, "-|", "tail -n 50000 $log" or die "Unable to open $log:$!\n"; open (OUTPUT,">/etc/squid/iplist.txt"); open (OUTPUT2,">/etc/squid/SuspendIpList.txt");
You should also die if neither of those could be opened:
open(OUTPUT, ">...") or die "can't create /etc/squid/iplist.txt: $!";
while (<$slog>){ # assigns each line in turn to $_
# use an array slice to select the fields we want
@data = (split ,$_)[1,4,10,5,7];
$hr = (split /:/ ,$data[0])[0];
$ip = "$data[1]";
Those three variables should all be declared with 'my'. Your line assigning to @data has a typo that hasn't effected you, but it might eventually.
my @data = (split)[1,4,10,5,7]; # why out of order?
my $hr = (split /:/, $data[0])[0];
my $ip = $data[1]; # no need to quote $data[1] hereif ($flag eq $data[2]) {
Where is $flag coming from?
if ($hr eq $hour) {
Where is $hour coming from?
Those two if statements can be combined into one, since you don't do anything if they aren't both true.
if ($flag eq $data[2] and $hr eq $hour) { foreach (/$data[2]/) {
$matches += 1 ;
}
I have a feeling this could lead to false positives. How do you know that 'PO' (or whatever else $data[2] might hold) won't appear in the URL, for instance? Perhaps this should just be
$matches++;
But where is $matches coming from?!
if ($matches > $blocklimit) {
Where does $blocklimit come from?!
$ip1 = "$data[1]/32";
Declare that with 'my'.
print OUTPUT "$matches,", "$hour, ","$ip1, ", "@data","\n";
You could just write that as
print OUTPUT "$matches, $hour, $data[1]/32 @data\n";
print OUTPUT2 "$ip1\n";
$matched = $matches;
$matches = 0;
Where did $matched come from?
}
}
}
}
close (OUTPUT);
close (OUTPUT2);
}
You should not use any variables in a function that you did not pass to it or create IN it.
-- Jeff "japhy" Pinyan % How can we ever be the sold short or RPI Acacia Brother #734 % the cheated, we who for every service http://www.perlmonks.org/ % have long ago been overpaid? http://princeton.pm.org/ % -- Meister Eckhart .
- Follow-Ups:
- Re: a little help...
- From: FamiLink Admin
- Re: a little help...
- References:
- a little help...
- From: FamiLink Admin
- a little help...
- Prev by Date: a little help...
- Next by Date: question about # of files in a directory
- Previous by thread: a little help...
- Next by thread: Re: a little help...
- Index(es):
Relevant Pages
|
|
