Hello Time Tim Wolak wrote: > > I'm working on a bit of code to parse a logfile, grab the IP's and put > them in the deny file. In the beginning of my code I'm grabbing all the > IP's in the deny file, putting them in an array to check against later > to make sure that IP is not already in the file. I can get all the > information I need but when I do a comparison of the IP's grabbed from > two files when there is a match is won't print the match! > > Any help would be great! > Thanks, > Tim > > #!/usr/bin/perl -w > > #use strict;
Uncomment this immediately! and use warnings; as well. > use IO::Handle; > my $logfile = "/var/log/messages"; > my $secv = "/var/log/secv"; > my $hosts = "/etc/hosts.deny"; > my $cody = "/etc/hosts.deny"; > my @boxes; > my $box; Best to declare $box at its point of use below. > open(LOG, "$logfile") || die "Cannot open logfile for reading: $!"; > open(SEC, ">$secv") || die "Can't open file!: $!"; > open(HOST, "$hosts") || die "Can't open file!: $!"; > #open(DEAD, ">$cody") || die "Can't open file!: $!"; It's considered bad form to put lone scalars inside quotes, as it's very unlikely to be necessary and could cause errors, depending on what's in the scalar: open LOG, $logfile or die "Cannot open logfile for reading: $!"; open SEC, '>', $secv or die "Can't open file!: $!"; open HOST, $hosts or die "Can't open file!: $!"; > > foreach (<HOST>) { > if($_ =~ /(\d+\.\d+\.\d+\.\d+)/) { A regex without an object string will match against $_ by default: if (/(\d+\.\d+\.\d+\.\d+)/) { : } > push (@boxes, $_); > } > else { > next; > } > } You're pushing the entire record from hosts.deny onto the array instead of just the IP addresses. This will include the terminating newline and maybe other stuff as well. That's OK as long as you know what you've got, but there's no need for the capturing parentheses in the regex. This looks tidier: foreach (<HOST>) { push @boxes, $_ if /\d+\.\d+\.\d+\.\d+/; } but if you want just the IP address: foreach (<HOST>) { push @boxes, $1 if /(\d+\.\d+\.\d+\.\d+)/; } > close HOST; > > while (<LOG>){ > if($_ =~/Failed password for invalid/) { Again, $_ is the default object. Use: if (/Failed password for invalid/) { : } > print SEC "Invalied user logon attempt!:$_\n"; > if(/(\d+\.\d+\.\d+\.\d+)/) { > $tim = $1; $tim needs declaring: my $tim = $1; > foreach $box (@boxes) { Declare $box here instead of at the top of the program: foreach my $box (@boxes) { : } > if($box =~ m/"$tim"/){ Do you intend the quotes? If the lines from the original hosts.deny file (in @boxes) have IP addresses in quotes then you're OK, but otherwise take them out. This is my best guess as to why your code isn't working. Also, you really need to escape the dots in $tim, otherwise they'll match any character instead of literal dots. if ($box =~ /\Q$tim/) { : } > print "Match:$tim\n" > } else { > print "No Match:$box\n"; > } > } > } > > } > } Finally, those nested blocks are confusing. Tidy things up with 'next if' stuff: while (<LOG>) { next unless /Failed password for invalid/; print SEC "Invalied user logon attempt!:$_\n"; next unless /(\d+\.\d+\.\d+\.\d+)/; my $tim = $1; foreach $box (@boxes) { if ($box =~ m/\Q$tim/){ print "Match:$tim\n" } else { print "No Match:$box\n"; } } } That's a lot of comments but not much of an answer! Without seeing your data I can only guess, but it seems to me that the most likley cause o the failure to match is the double quotes around $tim in the loop. If they're not there in the deny file then the match will fail. HTH, Rob -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>