I would replace the file reading loops with while(my $line=<DHCPDCONF>) and while(my $line=<LEASES>) respectively. You could even do just while(<DHCPDCONF>) and drop the line$=~ parts from your regex's. This will save a lot of memory and also make the script faster. You could save the information you'll need to rewrite the config file in a hash which is faster and cheaper on your memory.

In the same spirit I would place the print DHCPDCONF line into the loop that generates the file content, and replace the loop with: while(my($device,$ip)=each %devices) and use $ip in the substitution regex. This again saves a lot of memory and is faster. I you really want to handle the existing file content, you could loop thru the file line by line, extract it's $devices key from the line, check to see if it exists in your hash with ip's, and if so perform a substitution on the line. All the while saving to a temporary file, which you later move over the original.

This should be faster, cleaner and ultimately more versatile. (And it might not hang...)

I hadn't realized that while loops were more efficient than for loops... I just liked the english-like syntax of the foreach loop :) I also usually try to avoid the $_ variable because early in my perl career I always had trouble keeping track of when it was modified.

Anyway, stylistic preferences aside... I made the foreach -> while loop changes you suggested and am getting the same results. With both the original and modified versions of the code the hangup point has moved to just after Marker 2.

I also decided to leave things alone and let the code run its course... and it does decide to continue after about 30 seconds. Which might at least be acceptable as this script will only be running as an hourly cronjob.

I'm also not sure if this really is a memory issue. Here's the output from top while the script is hung up:

PID COMMAND %CPU   TIME   #TH #PRTS #MREGS RPRVT  RSHRD  RSIZE  VSIZE
1279 perl   19.6%  0:02.28   1    10    21   524K  1.42M  1.62M  2.92M
[download]

As you can see, perl isn't using up much memory at all. For those of you wondering about the strange top format... this is on my ibook. Here's the output from top on a linux box:

PID USER PRI  NI  SIZE  RSS SHARE STAT %CPU %MEM   TIME COMMAND
16974 root  25   0  1696 1696  1128 R    51.5  0.1   0:02 ip_update.pl
[download]

Well, in the middle of writing this I finally understood what you meant by movind the print DHCPDCONF line and the temporary file... so I made those adjustments and, of course there is no hang. I'm still not sure why it was hanging in the first place, but I guess that is a moot point now.

Thanks for your help :)

Updated Code:

#!/usr/bin/perl

use strict;

#####################################################
# Update MAC-IP pairings for all registered devices #
#####################################################

my %devices;

# Open dhcpd.conf and initialize the %devices hash with all
# registered modems and cpes

open (DHCPDCONF, "dhcpd.conf") or die "Cannot read from file: $!";
while (<DHCPDCONF>)
{
    $devices{lc($1)} = '000.000.000.000'
      if (/#.*#.*#.*# Modem MAC: (\w+:\w+:\w+:\w+:\w+:\w+)/
             or /subclass ".*" 1:(\w+:\w+:\w+:\w+:\w+:\w+)/);
             
}
close DHCPDCONF;

# open leases file and match mac addresses in %devices with active ips
open (LEASES, "dhcpd.leases");
my ($current_ip, $state);
while (<LEASES>)
{

    $current_ip = $1 and next if /^\s*lease (\d+.\d+.\d+.\d+)/;
                  
    $state = $1 and next if /^\s*binding state (\w+)/;
    
    $devices{lc($1)} = $current_ip and
    print lc($1) . " - " . $devices{lc($1)} . "\n" and next if
        (/^\s*hardware ethernet (\w+:\w+:\w+:\w+:\w+:\w+)/
         and $state eq 'active' and exists $devices{lc($1)});    
}
close LEASES;

# substitute old ip with current value
open (DHCPDCONF, "dhcpd.conf") or die "Cannot read from file: $!";
open (DHCPDCONFNEW, ">dhcpd.conf.new") or die "Cannot create file: $!"
+;
while( <DHCPDCONF>)
{
    my $device = lc($1) if /#.*#.*#.*# Modem MAC: (\w+:\w+:\w+:\w+:\w+
+:\w+)/;
    
    s/(#.*#.*# Modem IP: ).*( # Modem MAC: $device)/$1$devices{$device
+}$2/i;
    
    print DHCPDCONFNEW;
    
}
close DHCPDCONF;
close DHCPDCONFNEW;
system("cp dhcpd.conf.new dhcpd.conf");

######## Update LDAP database

# not implemented yet.
[download]

Looks good!

The reason the old code seemed to hang is probably because of the way the for loop made things happen;

The 'keys' at the loop initializer extracted 2300+ keys from a hash. That's hefty operation 1.

Then 2300+ times, you extract a single entry from the hash with the key you named (hefty op 2), and perform a substitution regex with match variabels like $1/$2 (hefty op 3).

And you store it in a stringus humangeous. (Hefty op 4).

Now you're doing it nice and and gentle, one line at a time... :)

About the while instead of foreach; the 'each' operator is a very gentle way to handle hashes, because it just traverses the hash instead of performing an operation which involves the entire thing. And you can use it to get the key and value in one swoop. It might not look as 'english' as the foreach construct, but it's worth it!

Anyway, glad I could help... Happy coding!

That's not normal. What does this snippet do?

my $dhcpdconf = '';

open (DHCPDCONF, "dhcpd.conf") or die "Cannot read from file: $!";
while (<DHCPDCONF>)
{
    $devices{lc($1)} = ''
      if (/#.*#.*#.*# Modem MAC: (\w+:\w+:\w+:\w+:\w+:\w+)/
             or /subclass ".*" 1:(\w+:\w+:\w+:\w+:\w+:\w+)/);
             
    $dhcpdconf .= $_;
}

print "--\n$dhcpdconf\n--\n";
[download]

Thanks for your reply... that snippet works just fine as a standalone script, but when incorporated into the existing code it hangs at the same spot.

I should also add that I have tried this on two different machines with the same results.

-derby

Thanks for the welcome :)
I also don't blame them for leaving me out... and to tell you the truth I'm a little relieved. If I'm not in the movie they can't butcher my character :)