How can I write Perl that doesn't look like C? [closed]

Question

My co-workers complain that my Perl looks too much like C, which is natural since I program in C most of the time, and Perl just a bit. Here's my latest effort. I'm interest in Perl that is easy to understand. I'm a bit of a Perl critic, and have little tolerance for cryptic Perl. But with readability in mind, how could the following code be more Perlish?

It's goal is to do a traffic analysis and find which IP addresses are within the ranges given in the file "ips". Here's my effort:

#!/usr/bin/perl -w

# Process the files named in the arguments, which will contain lists of IP addresses, and see if 
# any of them are in the ranges spelled out in the local file "ip", which has contents of the
# form start-dotted-quad-ip-address,end-dotted-quad-ip_address,stuff_to_be_ignored
use English;


open(IPS,"ips") or die "Can't open 'ips' $OS_ERROR";

# Increment a dotted-quad ip address
# Ignore the fact that part1 could get erroneously large.
sub increment {
    $ip = shift;

    my ($part_1, $part_2, $part_3, $part_4) = split (/\./, $ip);
    $part_4++;
    if ( $part_4 > 255 ) {
        $part_4 = 0;
        ($part_3++);
        if ( $part_3 > 255 ) {
            $part_3 = 0;
            ($part_2++);
            if ( $part_2 > 255 ) {
                $part_2 = 0;
                ($part_1++);
            }
        }
   }   
    return ("$part_1.$part_2.$part_3.$part_4");
}

# Compare two dotted-quad ip addresses.
sub is_less_than {
    $left = shift;
    $right = shift;

    my ($left_part_1, $left_part_2, $left_part_3, $left_part_4)     = split (/\./, $left);
    my ($right_part_1, $right_part_2, $right_part_3, $right_part_4) = split (/\./, $right);


    if  ($left_part_1 != $right_part_1 ) { 
        return ($left_part_1 < $right_part_1);
    }   
    if  ($left_part_2 != $right_part_2 ) { 
        return ($left_part_2 < $right_part_2);
    }   
    if  ($left_part_3 != $right_part_3 ) { 
        return ($left_part_3 < $right_part_3);
    }
    if  ($left_part_4 != $right_part_4 ) {
        return ($left_part_4 < $right_part_4);
    }
    return (false);  # They're equal
}

my %addresses;
# Parse all the ip addresses and record them in a hash.   
while (<IPS>) {
    my ($ip, $end_ip, $junk) = split /,/;
    while (is_less_than($ip, $end_ip) ) {
        $addresses{$ip}=1;
        $ip = increment($ip);
    }
}

# print IP addresses in any of the found ranges

foreach (@ARGV) {
    open(TRAFFIC, $_) or die "Can't open $_ $OS_ERROR";
    while (<TRAFFIC> ) {
        chomp;
        if (defined $addresses{$_}) {
            print "$_\n";
        }
    }
    close (TRAFFIC);

}

Gratitude

I kind of like your style. All of the code that I've seen designed in the Perl 'style' has been very cryptic, as you suggest. – Guarneri

I was contemplating answering that your code is too legible to be real Perl ;-) jokingly of course. – Bashkir

Do you avoid using any data structures because of lack of experience in Perl? I find repeated operations on similar octets not very readable. And modules from CPAN would certainly work more efficiently. – Carton

... but that said, I have to say I'm impressed with the general lack of unnecessarily-cryptic rubbish in the posted solutions. Good job people! – Benares

Answer 1

Sometimes the most Perlish thing to do is to turn to CPAN instead of writing any code at all.

Here is a quick and dirty example using Net::CIDR::Lite and Net::IP::Match::Regexp:

#!/path/to/perl

use strict;
use warnings;

use English;
use IO::File;
use Net::CIDR::Lite;
use Net::IP::Match::Regexp qw(create_iprange_regexp match_ip);


my $cidr = Net::CIDR::Lite->new();

my $ips_fh = IO::File->new();

$ips_fh->open("ips") or die "Can't open 'ips': $OS_ERROR";

while (my $line = <$ips_fh>) {

    chomp $line;

    my ($start, $end) = split /,/, $line;

    my $range = join('-', $start, $end);

    $cidr->add_range($range);

}

$ips_fh->close();

my $regexp = create_iprange_regexp($cidr->list());

foreach my $traffic_fn (@ARGV) {

    my $traffic_fh = IO::File->new();

    $traffic_fh->open($traffic_fn) or die "Can't open '$traffic_fh': $OS_ERROR";

    while (my $ip_address = <$traffic_fh>) {

        chomp $ip_address;

        if (match_ip($ip_address, $regexp)) {
            print $ip_address, "\n";
        }     

    }

    $traffic_fh->close();

}

DISCLAIMER: I just banged that out, it's had minimal testing and no benchmarking. Sanity checks, error handling and comments omitted to keep the line count down. I didn't scrimp on the whitespace, though.

As for your code: There is no need to define your functions before you use them.

Duchamp

Great suggestion: Care to flesh this out with some actual code? – Gratitude

I wrote Net::CIDR::Lite, and I'm not sure how I'd use the library for this. There is an ip increment function buried in there, but it's not exposed as a public method, and was written that way to work with IPv4 or IPv6 addresses. – Provitamin

I didn't scroll down on the code in the OP, so I missed most of the point, and I only saw the incrementing part...my bad. 8-) – Provitamin

...and now that I have thoroughly read the OP, I don't think you need Net::IP::Match::Regexp. I believe you can just use $cidr->find($ip_address) from Net::CIDR::Lite. – Provitamin

...though Net::CIDR::Lite might be slower than Net::IP::Match::Regexp at the finding...I haven't benchmarked, and I don't know how much the speed matters for this. – Provitamin

Net::IP::Match::Regexp comes with a benchmarking script. I added Net::CIDR::Lite to the benchmark, and it comes out about 2x slower (or less) than Net::IP::Match::Regexp, and sometimes faster than Net::IP::Match::XS. YMMV. – Provitamin

...sorry for the constant addendums...but by 2x slower, I mean specifically at worst 0.3 seconds longer on my machine testing 10,000 ip addresses against 1000 networks. And only 0.1 seconds longer if you don't care which network the ip was found in. – Provitamin

Answer 2

From years of seeing Perl code written by C programmers, here's some generic advice:

Use hashes. Use lists. USE HASHES! USE LISTS! Use list operations (map, grep, split, join), especially for small loops. Don't use fancy list algorithms; pop, splice, push, shift and unshift are cheaper. Don't use trees; hashes are cheaper. Hashes are cheap, make them, use them and throw them out! Use the iterator for loop, not the 3-arg one. Don't call things $var1, $var2, $var3; use a list instead. Don't call things $var_foo, $var_bar, $var_baz; use a hash instead. Use $foo ||= "default". Don't use $_ if you have to type it.

Don't use prototypes, IT'S A TRAP!!

Use regexes, not substr() or index(). Love regexes. Use the /x modifier to make them readable.

Write statement if $foo when you want a block-less conditional. There's almost always a better way to write a nested condition: try recursion, try a loop, try a hash.

Declare variables when you need them, not at the top of the subroutine. use strict. use warnings, and fix them all. use diagnostics. Write tests. Write POD.

Use CPAN. Use CPAN! USE CPAN! Someone's probably already done it, better.

Run perlcritic. Run it with --brutal just for kicks. Run perltidy. Think about why you do everything. Change your style.

Use the time not spent fighting the language and debugging memory allocation to improve your code.

Ask questions. Take style commentary on your code graciously. Go to a Perl Mongers meeting. Go onto perlmonks.org. Go to YAPC or a Perl Workshop. Your Perl knowledge will grow by leaps and bounds.

Handley

Answer 3

Most of writing code to be "Perlish" would be taking advantage of the built-in functions in Perl.

For instance, this:

my ($part_1, $part_2, $part_3, $part_4) = split (/\./, $ip);
$part_4++;
if ( $part_4 > 255 ) {
    $part_4 = 0;
    ($part_3++);
    if ( $part_3 > 255 ) {
        $part_3 = 0;
        ($part_2++);
        if ( $part_2 > 255 ) {
            $part_2 = 0;
            ($part_1++);
        }
    }
}   

I would rewrite something like:

my @parts = split (/\./, $ip);

foreach my $part(reverse @parts){
  $part++;
  last unless ($part > 255 && !($part = 0));
}

That does what your code posted above does but is a little cleaner.

Are you sure the code does what you want though? Just to me it looks a little strange that you only move to the previous 'part' of the IP if the one after it is > 255.

Cureton

Add that you would "return join('.', @parts);" and that's an excellent answer. – Amatory

@ashgromnies: yes, that is correct. If the ip address starts at 192.156.255.255, the result should be 192.157.0.0 as in the questioner's code. – Thigmotropism

The ($part = 0) in the unless condition is scary! It's clever, but I think it's an example of stereotypically unmaintainable Perl. – Shanta

Thaks for this answer. Very clever code, but I have to agree with Miles. I think Damian Conway in PBP would not endorse the complexity of the unless expression (which he generally discourages people from using IIRC.) – Gratitude

There are regexs that do this in one line. Still, your code is nice. – Poise

Answer 4

Another example rewrite:

sub is_less_than {
    my $left = shift; # I'm sure you just "forgot" to put the my() here...
    my $right = shift;

    my ($left_part_1, $left_part_2, $left_part_3, $left_part_4)     = split (/\./, $left);
    my ($right_part_1, $right_part_2, $right_part_3, $right_part_4) = split (/\./, $right);


    if  ($left_part_1 != $right_part_1 ) { 
        return ($left_part_1 < $right_part_1);
    }   
    if  ($left_part_2 != $right_part_2 ) { 
        return ($left_part_2 < $right_part_2);
    }   
    if  ($left_part_3 != $right_part_3 ) { 
        return ($left_part_3 < $right_part_3);
    }
    if  ($left_part_4 != $right_part_4 ) {
        return ($left_part_4 < $right_part_4);
    }
    return (false);  # They're equal
}

To this:

sub is_less_than {
    my @left = split(/\./, shift);
    my @right = split(/\./, shift);

    # one way to do it...
    for(0 .. 3) {
        if($left[$_] != $right[$_]) {
            return $left[$_] < $right[$_];
        }
    }

    # another way to do it - let's avoid so much indentation...
    for(0 .. 3) {
        return $left[$_] < $right[$_] if $left[$_] != $right[$_];
    }

    # yet another way to do it - classic Perl unreadable one-liner...
    $left[$_] == $right[$_] or return $left[$_] < $right[$_] for 0 .. 3;

    # just a note - that last one uses the short-circuit logic to condense
    # the if() statement to one line, so the for() can be added on the end.
    # Perl doesn't allow things like do_this() if(cond) for(0 .. 3); You
    # can only postfix one conditional. This is a workaround. Always use
    # 'and' or 'or' in these spots, because they have the lowest precedence.

    return 0 == 1; # false is not a keyword, or a boolean value.
    # though honestly, it wouldn't hurt to just return 0 or "" or undef()
}

Also, here:

my ($ip, $end_ip, $junk) = split /,/;

$junk might need to be @junk to capture all the junk, or you can probably leave it off - if you assign an unknown-sized array to an "array" of two elements, it will silently discard all the extra stuff. So

my($ip, $end_ip) = split /,/;

And here:

foreach (@ARGV) {
    open(TRAFFIC, $_) or die "Can't open $_ $OS_ERROR";
    while (<TRAFFIC> ) {
        chomp;
        if (defined $addresses{$_}) {
            print "$_\n";
        }
    }
    close (TRAFFIC);
}

Instead of TRAFFIC, use a variable to store the filehandle. Also, in general, you should use exists() to check if a hash element exists, rather than defined() - it might exist but be set to undef (this shouldn't happen in your program, but it's a nice habit for when your program gets more complicated):

foreach (@ARGV) {
    open(my $traffic, $_) or die "Can't open $_ $OS_ERROR";
    while (<$traffic> ) {
        chomp;
        print "$_\n" if exists $addresses{$_};
    }
    # $traffic goes out of scope, and implicitly closes
}

Of course, you could also use Perl's wonderful <> operator, which opens each element of @ARGV for reading, and acts as a filehandle that iterates through them:

while(<>) {
    chomp;
    print "$_\n" if exists $addresses{$_};
}

As has been noted before, try to avoid useing English unless you use English qw( -no_match_vars ); to avoid the significant performance penalty of those evil match_vars in there. And as hasn't been noted yet, but should be...

ALWAYS ALWAYS ALWAYS always use strict; and use warnings; or else Larry Wall will descend from heaven and break your code. I see you have -w - this is enough, because even off of Unix, Perl parses the shebang line, and will find your -w and will use warnings; like it should. However, you need to use strict;. This will catch a lot of serious errors in your code, like not declaring variables with my or using false as a language keyword.

Making your code work under strict as well as warnings will result in MUCH cleaner code that never breaks for reasons you can't figure out. You'll spend hours at the debugger debugging and you'll probably end up using strict and warnings anyway just to figure out what the errors are. Only remove them if (and only if) your code is finished and you're releasing it and it never generates any errors.

Amatory

Great answer. One nit to pick, though. Its better to use "if exists $addresses{$_}" than to check if the value is defined. Also, I would seriously consider using Hash::Util's lock_hash method to prevent accidental changes to my lookup hash, once it was fully loaded. – Heaviside

Answer 5

While doing this certainly is one way to do it in Perl.

use strict;
use warnings;

my $new_ip;
{
  my @parts = split ('\.', $ip);

  foreach my $part(reverse @parts){
    $part++;

    if( $part > 255 ){
      $part = 0;
      next;
    }else{
      last;
    }
  }
  $new_ip = join '.', reverse @parts;
}

This is how I would actually implement it.

use NetAddr::IP;

my $new_ip = ''.(NetAddr::IP->new($ip,0) + 1) or die;

Mcquillin

Answer 6

I can't say that this solution will make your program more Perl-ish, but it might simplify your algorithm.

Rather than treating an IP address as a dotted-quad, base-256 number which needs the nested-if structure to implement the increment function, consider an IP address to be a 32-bit integer. Convert an IP of the form a.b.c.d into an integer with this (not tested):

sub ip2int {
    my $ip = shift;
    if ($ip =~ /(\d+)\.(\d+)\.(\d+)\.(\d+)/) {
        return ($1 << 24) + ($2 << 16) + ($3 << 8) + $4;
    } else {
        return undef;
    }
}

Now it's easy to determine if an IP falls between two endpoint IPs. Just do simple integer arithmetic and comparisons.

$begin = "192.168.5.0";
$end = "192.168.10.255";
$target = "192.168.6.2";
if (ip2int($target) >= ip2int($begin) && ip2int($target) <= ip2int($end)) {
    print "$target is between $begin and $end\n";
} else {
    print "$target is not in range\n";
}

Clique

or use the inet_aton function from Socket.pm – Vast

Yes, that would be better. I'd forgotten about that module. – Clique

Answer 7

Tell your coworkers that their perl looks too much like line noise. Please don't obfuscate your code just for the sake of obfuscation - it's asinine development goals like that which give perl such a bad reputation for being unreadable, when it's really bad programmers (apparently, like your coworkers) who write sloppy code. Nicely structured, indented, and logical code is a good thing. C is a good thing.

Seriously, though - the best place to figure out how to write perl is in the O'Reilly "Perl Best Practices", by Damian Conway. It tells you how he thinks you should do things, and he always gives good reasons for his position as well as occasionally giving good reasons to disagree. I do disagree with him on some points, but his reasoning is sound. The odds that you work with anyone who knows perl better than Mr. Conway are pretty slim, and having a printed book (or at least a Safari subscription) gives you some more solid backing for your arguments. Pick up a copy of the Perl Cookbook while you're at it, as looking at code examples for solving common problems should get you on the right track. I hate to say "buy the book", but those are exceptionally good books that any perl developer should read.

With regards to your specific code, you're using foreach, $_, split with no parens, shift, etc. It looks plenty perl-ish to my eyes - which have been developing with perl for quite a while. One note, though - I hate the English module. If you must use it, do it like use English qw( -no_match_vars );. The match_vars option slows down regexp parsing measurably, and the $PREMATCH / $POSTMATCH variables it provides aren't usually useful.

Clubbable

For the record, I think your code should look roughly like your coworkers to increase how well everyone on the project can read everyone else's code. Imagine if everyone in the project wrote code in a different language (style). No one would be able to fix anyone else's code. – Amatory

@Chris While I agree with the uniformity sentiment, you really should mention that there should be some kind of standard everyone has to live to. Whether it's LCD or something from the Tech Lead, it should be something everyone agrees to. Otherwise making your code look like coworkers code loses when the first guy to touch everything sets the standard, and it just so happens that his/her code is terrible. Now the standard is terrible. This is probably a no-brainer for most people, but just throwing it in for the sake of completion. – Death

Answer 8

I know exactly how you feel. My first language was FORTRAN and like a good FORTRAN programmer, I wrote FORTRAN in every language since :).

I have this really wonderful book Effective Perl Programming that I keep re-reading every now and then. Especially a chapter called "Idiomatic Perl". Here are a few things I use to keep my Perl looking like Perl: List Operators like for map and grep, slices and hash slices, the quote operators.

Another thing that keeps my Perl from looking like FORTRAN/C is a regular reading of module sources especially those of the masters.

Ambi

Yeah - Effective Perl Programming is awesome. I haven't read it for many years, but it was very important in my development as a Perl programmer. – Playsuit

Answer 9

While this would work:

use strict;
use warnings;
use 5.010;

use NetAddr::IP;

my %addresses;
# Parse all the ip addresses and record them in a hash.
{
  open( my $ips_file, '<', 'ips') or die;

  local $_; # or my $_ on Perl 5.10 or later
  while( my $line = <$ips_file> ){
    my ($ip, $end_ip) = split ',', $line;
    next unless $ip and $end_ip;

    $ip     = NetAddr::IP->new( $ip, 0 ) or die;
    $end_ip = NetAddr::IP->new( $end_ip ) or die;
    while( $ip <= $end_ip ){
      $addresses{$ip->addr} = 1;
      $ip++;
    }
  }
  close $ips_file
}

# print IP addresses in any of the found ranges
use English;

for my $arg (@ARGV) {
  open(my $traffic, '<',$arg) or die "Can't open $arg $OS_ERROR";
  while( my $ip = <$traffic> ){
    chomp $ip;
    if( $addresses{$ip} ){
      say $ip
    }
  }
  close ($traffic);
}

I would if possible use netmasks, because it gets even simpler:

use Modern::Perl;
use NetAddr::IP;

my @addresses;
{
  open( my $file, '<', 'ips') or die;

  while( (my $ip = <$file>) =~ s(,.*){} ){
    next unless $ip;
    $ip = NetAddr::IP->new( $ip ) or die;
    push @addresses, $ip
  }

  close $file
}


for my $filename (@ARGV) {
  open( my $traffic, '<', $filename )
    or die "Can't open $filename";

  while( my $ip = <$traffic> ) {
    chomp $ip;
    next unless $ip;

    $ip = NetAddr::IP->new($ip) or next; # skip line on error
    my @match;
    for my $cmp ( @addresses ){
      if( $ip->within($cmp) ){
        push @match, $cmp;
        #last;
      }
    }

    say "$ip => @match" if @match;

    say "# no match for $ip" unless @match;
  }
  close ($traffic);
}

Test ips file:

192.168.0.1/24
192.168.0.0
0:0:0:0:0:0:C0A8:0/128

Test traffic file:

192.168.1.0
192.168.0.0
192.168.0.5

Output:

# no match for 192.168.1.0/32
192.168.0.0/32 => 192.168.0.1/24 192.168.0.0/32 0:0:0:0:0:0:C0A8:0/128
192.168.0.5/32 => 192.168.0.1/24

Mcquillin

Answer 10

Instead of doing this :

if  ($left_part_1 != $right_part_1 ) { 
    return ($left_part_1 < $right_part_1);
}

you could do this :

return $left_part_1 < $right_part_1 if($left_part_1 != $right_part_1);

Also, you could use the Fatal module, to avoid checking stuff for errors.

Bini

I think that autodie is the new Fatal - though that seems to be only as of 5.10. – Hhd

Answer 11

The only criteria I use for "how my code looks" is how easy it is to read and understand the purpose of the code (especially by programmers unfamiliar with Perl), not whether it follows a particular style.

If a Perl language feature makes some logic easier to understand then I use it, if not I don't - even if it can do it in less code.

Your co-workers may think my code is extremely "un perl-ish", but I'll bet they understood exactly what the code is doing and could modify it to fix / extend it without any trouble:

my version:

#******************************************************************************
# Load the allowable ranges into a hash
#******************************************************************************
my %ipRanges = loadIPAddressFile("../conf/ip.cfg");

#*****************************************************************************
# Get the IP to check on the command line
#*****************************************************************************
my ( $in_ip_address ) = @ARGV;

# Convert it to number for comparison
my $ipToCheckNum = 1 * sprintf("%03d%03d%03d%03d", split(/\./, $in_ip_address));

#*****************************************************************************
# Loop through the ranges and see if the number is in any of them
#*****************************************************************************
my $startIp;
my $endIp;
my $msg = "IP [$in_ip_address] is not in range.\n";

foreach $startIp (keys(%ipRanges))
   {
   $endIp = $ipRanges{$startIp};

   if ( $startIp <= $ipToCheckNum and $endIp >= $ipToCheckNum ) 
      {
      $msg = "IP [$in_ip_address] is in range [$startIp] to [$endIp]\n";
      }
   }

print $msg;

#******************************************************************************
# Function: loadIPAddressFile()
#   Author: Ron Savage
#     Date: 04/10/2009
# 
# Description:
# loads the allowable IP address ranges into a hash from the specified file.
# Hash key is the starting value of the range, value is the end of the range.
#******************************************************************************
sub loadIPAddressFile
   {
   my $ipFileHandle;
   my $startIP;
   my $endIP;
   my $startIPnum;
   my $endIPnum;
   my %rangeList;

   #***************************************************************************
   # Get the arguments sent
   #***************************************************************************
   my ( $ipFile ) = @_;

   if ( open($ipFileHandle, "< $ipFile") )
      {
      while (<$ipFileHandle>)
         {
         ( $startIP, $endIP ) = split(/\,/, $_ );

         # Convert them to numbers for comparison
         $startIPnum = 1 * sprintf("%03d%03d%03d%03d", split(/\./, $startIP));
         $endIPnum   = 1 * sprintf("%03d%03d%03d%03d", split(/\./, $endIP));

         $rangeList{$startIPnum} = $endIPnum;
         }

      close($ipFileHandle);
      }
   else
      {
      print "Couldn't open [$ipFile].\n";
      }

   return(%rangeList);
   }

(Note: the extra "#" lines are in there to preserve my freakin' spacing, which always gets whacked when posting code here)

Marie

I don't know why your spacing gets out of whack on the code blocks. That shouldn't happen. In fact, I've never seen it happen. – Amatory

And, not to say you're wrong, but I think most Perlites would consider your bracketing style to be straight from Satan himself (or Guido van Rossum :P ). Seriously, GNU-ish-style indentation in Perl? Most people would use K&R style or something similar. But whatever floats your metaphorical boat. – Amatory

The "blank line space" removal may be IE browser related, initially the blank lines show up in the "view" panel - then if I pause for a few moments or post it ... they get removed (in IE). irritating - much. :-) – Marie

I was going to suggest that that might be the problem, but didn't want to come off as a condescending Firefox fan. Which I totally am. – Amatory

Your point about readability being paramount is very good. The code is clean and understandable. The one flaw is the lack of 'use strict' and 'use warnings'. – Heaviside

I do actually use strict and warnings, but they are up at the top of the code with my header comment block and got chopped to save space. :-) – Marie

@Chris Lutz: I don't care for it, but it's a valid indentation style. perltidy even has an option for it (-bli). OF course, with perltidy, you can change it to (almost) any style you like :-) – Provitamin

Answer 12

This is probably more like C, but is also more simple:

use Socket qw(inet_aton inet_ntoa);

my $ip = ("192.156.255.255");

my $ip_1 = inet_ntoa(pack("N", unpack("N", inet_aton($ip))+1));
print "$ip $ip_1\n";

Update: I posted this before reading all of the code in the question. The code here just does the incrementing of the ip address.

Provitamin

Answer 13

This is a really good question, and the answers are superb.

I usually prefer the approach taught in the book "Minimal Perl" which emphasizes perl's command line switches, like -n to form an implicit line processing loop, -l to automatically chomp and append record separators, etc.

Since none of the answers showcase this, I thought I'd contribute with a fairly short alternative:

#!/usr/bin/perl -wln
use strict;
use Socket qw< inet_aton >;

our @ip_ranges;
INIT {
    open my $fh, '<', 'ips' or die "Couldn't open 'ips': $!";
    while (<$fh>) {
        my ($from, $to) = split/,/;
        push @ip_ranges, {  from => inet_aton($from),    
                            to   => inet_aton($to) };
    }
}

sub in_range {
    my $ip = shift;
    my $n  = inet_aton($ip);
    !!grep { $_->{from} le $n le $_->{to} } @ip_ranges;
}

print if in_range($_);

I would argue that this is fairly readable, and quite perlish.

Some things to note:

• -n wraps the code in a line processing block: while(<>){...}
  • subroutines are 'hoisted' out of this loop, so they only get defined once
• -l chomps lines read by <>, and appends a record separator (normally \n) to print statements
• lexical filehandles are closed automatically when they go out of scope
• unlike BEGIN blocks, INIT blocks are not executed during compilation and are therefore a better fit in this case, since the block is accessing the file-system.
• inet_aton returns binary data as a string, so we compare using le (less than or equal)
• grep returns a list, which in scalar context is coerced to the number of elements.
• numeric value 0 is false and any other number is true in boolean context
• !! is a perl idiom that forces boolean context.

---

Since this is perl, here is a very stupid one-liner:

perl -MSocket -lne 'INIT{open FH,"ips"; @r=map [map inet_aton($_),split/,/],<FH>;} $N=inet_aton($_); print if grep $_->[0] le $N le $_->[1], @r;' traffic

Hypogastrium