Re: Array of Hashes

Rob Dixon wrote:
>
> Hi Paul and welcome to the list.
>
> I can see a few things wrong with your code, but I have only a Windows
> machine so cannot test any changes I am suggestion so please beware.
>
> The reason you get the marked line in your output is because that is
> what you have written. This loop
>
> open(EXT,"/usr/sbin/pvdisplay $PV|");
> while(<EXT>) {
>   print $_;
>   if (/Free PE/) {
>     $AllocatedPE = (split(/\s+/, <EXT>))[2];
>     $rec->{$PV} = $AllocatedPE;
>     print "$PV ";
>     print $rec->{$PV};
>     print " \n";
>     push @vggroup, $rec;
>   }
> }
>
> reads a line from the pvdisplay output and checks whether it contains
> 'Free PE'. If it does, this
>
> $AllocatedPE = (split(/\s+/, <EXT>))[2];
>
> reads another line (the 'Allocated PE' line), splits it on white space
> and puts the third field - the extent - into $AllocatedPE. The code then
> goes on to add a single hash element to $rec and then print the volume
> name and extent.
>
> The line read within the call to split is never displayed, and the loop
> goes on to read and display the line after it in the while loop.
>
> Another problem is that you are pushing the same hash reference $rec
> onto @vggroup several times. There is no need for the array, as the hash
> holds the information on all the volumes in the group on its own.
>
> Other advice that may help is:
>
> - Perl variables generally use lower-case letters and underscores, and
> should be declared at the point of first use
>
> - Lexical file handles should be used with the three-argument form of
> open, and the status of /all/ opens should be checked before the file
> handle is used
>
> - By default the split operator splits $_ on whitespace, so split on its
> own returns a list of separate non-whitespace fields found in $_. This
> is usually what is wanted, as is the case here
>
> - There may well be a module already written to do this. It is worh a
> look at http://search.cpan.org unless your purpose is to learn the language
>
> Below is a refactored version of your program taking these things into
> account. As I explained I am unable to test it so I can only confirm
> that it compiles.
>
> HTH,
>
> Rob
>
>
> use strict;
> use warnings;
>
> open my $cmd, '-|', "/usr/sbin/vgdisplay -v vg03" or die $!;

You might as well go all the way:

open my $cmd, '-|', '/usr/sbin/vgdisplay', '-v', 'vg03'
     or die "Cannot open pipe from vgdisplay because: $!";


> my @pv;
>
> while (<$cmd>) {
>   if (/PV Name/) {
>     my $pv = (split)[2];
>     push @pv, $pv;
>   }
> }

And don't forget to verify that the pipe closed correctly:

close $cmd or warn $! ? "Error closing vgdisplay pipe: $!"
                       : "Exit status $? from vgdisplay";


Another way to populate @pv:

my @pv = `/usr/sbin/vgdisplay -v vg03` =~ /PV Name\s+(\S+)/g;


> print "@pv\n";
> my $rec;
>
> foreach my $pv (@pv) {
>   open my $ext, '-|', "/usr/sbin/pvdisplay $pv" or die $!;

     open my $ext, '-|', '/usr/sbin/pvdisplay', $pv
       or die "Cannot open pipe from pvdisplay because: $!";


>   while(<$ext>) {
>     print;
>     if (/Allocated PE/) {
>       my $allocated_pe = (split)[2];
>       $rec->{$pv} = $allocated_pe;
>       print "$pv => $allocated_pe\n";
>     }
>   }

    close $ext or warn $! ? "Error closing pvdisplay pipe: $!"
                          : "Exit status $? from pvdisplay";


> }
>
> use Data::Dumper;
> print Dumper $rec;



John
-- 
Any intelligent fool can make things bigger and
more complex... It takes a touch of genius -
and a lot of courage to move in the opposite
direction.                   -- Albert Einstein

• Array of Hashes by Paul.G
• Re: Array of Hashes by Rob Dixon
• Re: Array of Hashes by John W. Krahn
• Re: Array of Hashes by Paul.G
• Re: Array of Hashes - problem solved by Paul.G