Need advices for my script

[al0]

Hi,

I have made a script where I can extract lines from my apache logfile based on ip adresses. As I'm quite new to perl I know that the script I have made isnt efficient at all. So I wonder if anyone could help me a bit and give som tips how to do it better.

#!/usr/bin/perl
use warnings;
use Getopt::Std;
use Getopt::Long;

$match="match.log";
$error="error.log";
GetOptions (\%opts, 'i=s','l=s');

if(defined $opts{'i'}) {
	&validate($opts{'i'});
	if($ok==1) {
		&read_from_file();		
	}
	else {
		print "\n",$opts{'i'}," is an invalid ip\n";
	}
	
}

if(defined $opts{'l'}) {
	&validate($opts{'l'});
	if($ok==1) {
		&read_from_file();
	}
	else {
		print "\n",$opts{'i'}," is an invalid ip\n";
	}
	
}

Validate IP-address
sub validate {
	$ok=();
	$ip1 = qr/([1-9]?[0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])/;
	$ip2 = qr/^($ip1\.){3}$ip1$/;
	$validate=shift;	
	if($validate=~ /$ip2/) {
		$ok=1;
	}
	else {
		$ok=0;
	}
	
	
}

Open the apache logfile. Split on whitespace because every new line in the logfiles starts with the ip-address.
 
sub read_from_file {
open(FILE,$ARGV[0]);
while(<FILE>) {
	chomp(@logg=$_);			
	foreach $logg (@logg) {	
		@ips=split (/\s/);
		&validate($ips[0]);
		if(defined $opts{'i'}) {
			&match_ip($opts{'i'},$ips[0]);
		}
		elsif(defined $opts{'l'}) {
			&match_not_ip($opts{'l'},$ips[0]);
		}									
	}			
}		
close (FILE);
}

If IP is ok match the IP from the user with the IP in the logfile and print it to file. If IP is not correct call subroutine output and print it.

sub match_ip {
	$matchip=shift;
	$errorip=shift;
	if($ok==1){
		if ($logg=~/$matchip/) {
		@match=$logg;
		foreach (@match) {
			&output($_,$match);
		}
		}
	}
	else {
		@errors=grep {/$errorip/} $logg;
		foreach (@errors) {
			&output($_,$error);
		}
	}
}

The same as before but here its matching all but the IP the user specified. A lot of redundance in this two subroutines, dont know how to do it in another way.

sub match_not_ip {
	$matchip=shift;
	$errorip=shift;
	if($ok==1){
		if ($logg!~/$matchip/) {
		@match=$logg;
		foreach (@match) {
			&output($_,$match);
		}
		}
	}
	else {
		@errors=grep {/$errorip/} $logg;
		foreach (@errors) {
			&output($_,$error);
		}		
	}
}

Print to file. As for now I append to the file. But if I have already run the program before I get duplicates in the file which is bad.

sub output {
	$output=shift;
	$filename=shift;
	open(OUTPUT,">>$filename");
	select OUTPUT;
	print "$output\n";					
	select STDOUT;
	close OUTPUT;
}

 

[mbrooks]

The CPAN module Apache:arseLog is what you want to use to parse your logs.

http://search.cpan.org/~akira/Apache-ParseLog-1.02/ParseLog.pm

M. Brooks

http://mbrooks.info