views:

246

answers:

3

I am currently developing a piece of monitoring software that takes an input file of server names and ip addresses and creates a rudimentary database of information. I want to default some values as it processes the config file and it works fine for the first time round the loop but any subsequent entries get created with weird (well weird to me was the best way to describe it as it is probably correct and the code is wrong, as in the code is doing exactly what i have asked it to do but not necessarily what i want it to do).

the output from the code below looks like:

$VAR1 = [
      {
        'IPAddress' => '196.8.150.163',
        'Boxname' => 'MPLRDFDSOAK1',
        'CurrentStatusInfo' => {
                                 'LineHandlersRunning' => [
                                                            {
                                                              'NumberOfGaps' => 0,
                                                        'LineHandlerName' => 'DEFAULT',
                                                        'NumberOfCommLinkDowns' => 0,
                                                              'LineHandlerUpTime' => 0,
                                                              'MemoryUsage' => 0
                                                            }
                                                          ]
                               },
        'PreviousStatusInfo' => {
                                  'LineHandlersRunning' => 
                        $VAR1->[0]{'CurrentStatusInfo'}{'LineHandlersRunning'}[0]
                                                         ]
                                }
      },
      {
        'IPAddress' => '196.8.150.164',
        'Boxname' => 'MPLRDFDSOAK2',
        'CurrentStatusInfo' => {
                                 'LineHandlersRunning' => 
                        $VAR1->[0]{'CurrentStatusInfo'}{'LineHandlersRunning'}
                               },
        'PreviousStatusInfo' => {
                                  'LineHandlersRunning' => 
                        $VAR1->[0]{'PreviousStatusInfo'}{'LineHandlersRunning'}
                                }
      }
    ];


The following is the code:

#######################################################################################
# Version History                                                                     #
#######################################################################################
# example of the ini file
#box=>MPLRDFDSOAK1;ip=>196.8.150.163
#box=>MPLRDFDSOAK2;ip=>196.8.150.164

use strict;
use warnings;

# include the library to allow easy access to command line arguments
use Getopt::Long;

# include the data dumper utility
use Data::Dumper;

my $usageInstructions = "Some instructions\n";
my $showMeTheInstructions = "";
my $iniFileToReadIn = "";
my @boxes;

# read in the command line arguments
GetOptions( "ini=s"  => \$iniFileToReadIn,
  "H|h|?!" => \$showMeTheInstructions);

if ($showMeTheInstructions)
{
 print $usageInstructions;
 exit 0;
}

readInINIFileIn($iniFileToReadIn, \@boxes) if ($iniFileToReadIn ne "");

print Dumper(\@boxes);
print "\n\#\n\# END OF DATA DUMP\n\#\n\n";
exit 0;

#######################################################################################
# subroutine to read in the ini file and create the empty records for the boxes 
# specified
sub readInINIFileIn
{ 
 my ($iniFile, $pointerToBoxes) = @_;

 my $noCRLFOnString = "";

 # open the file
 open (ConfigFile, "<$iniFile") || die $!;

 # read in all the lines into an array
 my @configurationItems = <ConfigFile>;

 # close the file
 close (ConfigFile);

 # temporary record storage
 my %tempRecord;

 # create the defaults for all boxes
 my @LineHandlersRunning;

 my %tmpLineHandlerRunning = ( LineHandlerName => "DEFAULT", 
     LineHandlerUpTime => 0, 
     NumberOfCommLinkDowns => 0, 
     NumberOfGaps => 0, 
     MemoryUsage => 0 );

 push (@LineHandlersRunning, {%tmpLineHandlerRunning});

 my %CurrentStatusInfo;
 my %PreviousStatusInfo;

 push @{ $CurrentStatusInfo{'LineHandlersRunning'} },          @LineHandlersRunning;
 push @{ $PreviousStatusInfo{'LineHandlersRunning'} },         @LineHandlersRunning;

 # loop through the config file and create the defaults for the database of boxes
 foreach my $configLine (@configurationItems)
 {
  my @TokenisedLineFromFileItems = ();
  my @TokenisedLineFromFileNameValuePairs = ();

  # store parameters
  # each line will be ; separated then => separated, as in each one will have a number of items separated by ;'s and
  # each item will be be a name & value pair separated by =>'s
  @TokenisedLineFromFileItems = split(/;/,$configLine);

  # remove quote marks around the outside of each element of the newly created array
  s/^"|"$//g foreach @TokenisedLineFromFileItems;

  # create information in database record to add to boxes
  foreach my $NameValuePair (@TokenisedLineFromFileItems)
  {
   @TokenisedLineFromFileNameValuePairs = split(/=>/,$NameValuePair);
   $noCRLFOnString = $TokenisedLineFromFileNameValuePairs[1];
   $noCRLFOnString  =~ s/(\n|\r)//g;

   $tempRecord{'Boxname'} = $noCRLFOnString if ($TokenisedLineFromFileNameValuePairs[0] eq "box");
   $tempRecord{'IPAddress'} = $noCRLFOnString if ($TokenisedLineFromFileNameValuePairs[0] eq "ip");
  }

  # add all other defaults as blank
  $tempRecord{'CurrentStatusInfo'} = {%CurrentStatusInfo};
  $tempRecord{'PreviousStatusInfo'} = {%PreviousStatusInfo};

  push(@$pointerToBoxes, {%tempRecord});
 }
}
+3  A: 

I don't have the patience to wade through all of your code, but I'll bet your problem is related to this aspect of the Data::Dumper output:

$VAR1->[0]{'CurrentStatusInfo'}{'LineHandlersRunning'}[0]

In other words, your data structure contains a reference to other parts of the structure.

Perhaps you think you are making a copy of part of the data structure, but instead you are getting a shallow copy rather than a deep copy? For example, I'm suspicious of this code:

$tempRecord{'CurrentStatusInfo'} = {%CurrentStatusInfo};
$tempRecord{'PreviousStatusInfo'} = {%PreviousStatusInfo};

If indeed the problem is related to shallow copying, the Clone module might help.

FM
+2  A: 

Use lexical filehandles, declare variables in the smallest possible scope. I do not know what your problem is, but it is most likely caused by some variable persisting longer than you think it does.

Sinan Ünür
+1  A: 

I'm guessing it's because these two lines end up pushing the same hash reference into two locations - so if you alter the hashref contents in one location, the other will change as well which is probably not what you want for default values.

As FM pointed out, this is why you have the circular reference in your Dumper output.

If someone I'm waiting to get off the phone takes long enough i'll refactor your code for you.

Update: ok, so without knowing the full scenario it's hard to say if this is a sensible approach. certainly you should look at the various INI parsing modules in CPAN, but here is a very quick tweak of your code, leaving your existing logic structure in place:

use strict;
use warnings;

use Getopt::Long;
use Data::Dumper;

my $cmd_help = "Some instructions\n";
my $show_help = "";
my $ini_file_path = "";

# read in the command line arguments
GetOptions( "ini=s"  => \$ini_file_path,
      "H|h|?!" => \$show_help );

if ($show_help) {
    print $cmd_help;
    exit 0;
}

if (! -f $ini_file_path) {
    die "File '$ini_file_path' doesn't seem to exist.";
}

my $boxes = read_ini_file($ini_file_path);

print Dumper($boxes);

exit 0;

=head2 read_ini_file

read in the ini file and create the empty records for the boxes 

=cut

sub read_ini_file { 
    my ($ini_file) = @_;

    my @boxes;

    my @config_lines;
    {
     # consider using File::Slurp
     open (my $ini_fh, '<', $ini_file_path) || die $!;

     @config_lines = <$ini_fh>;
     chomp @config_lines; # remove \r\n

     # file handle will close when $ini_fh goes out of scope
    }

    # create the defaults for all boxes
    my %line_handlers_running_defaults = ( LineHandlerName => "DEFAULT", 
                LineHandlerUpTime => 0, 
                NumberOfCommLinkDowns => 0, 
                NumberOfGaps => 0, 
                MemoryUsage => 0 );

    # loop through the config file and create the defaults for the database of boxes
    foreach my $line (@config_lines) {

     my %record;

     my @token_pairs = map { s/^"//; s/^$//; $_ } split(/;/,$line);

     # create information in database record to add to boxes
     foreach my $pair (@token_pairs) {
      my ($key, $val) = split(/=>/,$pair);

      $record{Boxname} = $val if $key eq "box";
      $record{IPAddress} = $val if $key eq "ip";
     }

     # add all other defaults as blank
     $record{CurrentStatusInfo} = { LineHandlersRunning => [{%line_handlers_running_defaults}] };
     $record{PreviousStatusInfo} = { LineHandlersRunning => [{%line_handlers_running_defaults}] };

     push @boxes, \%record;
    }

    return \@boxes;
}

gives this output:

$VAR1 = [
      {
    'IPAddress' => '196.8.150.163',
    'CurrentStatusInfo' => {
                 'LineHandlersRunning' => [
                                {
                                  'NumberOfGaps' => 0,
                                  'LineHandlerName' => 'DEFAULT',
                                  'NumberOfCommLinkDowns' => 0,
                                  'LineHandlerUpTime' => 0,
                                  'MemoryUsage' => 0
                                }
                              ]
                   },
    'Boxname' => 'MPLRDFDSOAK1',
    'PreviousStatusInfo' => {
                  'LineHandlersRunning' => [
                                 {
                                   'NumberOfGaps' => 0,
                                   'LineHandlerName' => 'DEFAULT',
                                   'NumberOfCommLinkDowns' => 0,
                                   'LineHandlerUpTime' => 0,
                                   'MemoryUsage' => 0
                                 }
                               ]
                }
      },
      {
    'IPAddress' => '196.8.150.164',
    'CurrentStatusInfo' => {
                 'LineHandlersRunning' => [
                                {
                                  'NumberOfGaps' => 0,
                                  'LineHandlerName' => 'DEFAULT',
                                  'NumberOfCommLinkDowns' => 0,
                                  'LineHandlerUpTime' => 0,
                                  'MemoryUsage' => 0
                                }
                              ]
                   },
    'Boxname' => 'MPLRDFDSOAK2',
    'PreviousStatusInfo' => {
                  'LineHandlersRunning' => [
                                 {
                                   'NumberOfGaps' => 0,
                                   'LineHandlerName' => 'DEFAULT',
                                   'NumberOfCommLinkDowns' => 0,
                                   'LineHandlerUpTime' => 0,
                                   'MemoryUsage' => 0
                                 }
                               ]
                }
      }
    ];
Mark Aufflick
Thanks Markyour example and help was much appreciated as it showed me where i had gone wrong and actually gave an indication of how to rectify and, even more helpfully, simplify my code. Once again thank you and have a good day.kind regardsGlen
Glen