Beefy Boxes and Bandwidth Generously Provided by pair Networks
Clear questions and runnable code
get the best and fastest answer
 
PerlMonks  

please review my first module

by blueflashlight (Pilgrim)
on Jul 01, 2001 at 07:41 UTC ( #92983=perlquestion: print w/ replies, xml ) Need Help??
blueflashlight has asked for the wisdom of the Perl Monks concerning the following question:

Hi, Monks ... I'd like to get some feedback on my first module before I package it up and send it out into the world. It provides several functions to retrieve various information from an AIX system. It tests fine in the few environments that I have available, and there are no (perl) syntax errors, but I'm sure I've done some dumb things. I know I can count on the good folk here to point those dumb things out, and suggest a smart way to do it.

Thanks! --Sandy

package Aix::Sysinfo; require 5.6.0; use strict; use warnings; require Exporter; our @ISA = qw(Exporter); our %EXPORT_TAGS = ( 'all' => [ qw( ) ] ); our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } ); our @EXPORT = qw( hostname serial_number processor_count total_memory total_swap aix_version model_type processor_arch processor_speed hardware_info ); our $VERSION = '0.01'; #===================================================================== += $^O =~ /aix/i || die "This module only runs on AIX systems.\n"; sub hostname { chomp(my $hostname = `/usr/bin/hostname`); return $hostname }; sub serial_number { my $serial_num = substr(`/usr/bin/uname -m`, 2, 6); return $serial_num }; sub processor_count { my $num_proc; open (LSCFG, "/usr/sbin/lscfg -vp|"); while (<LSCFG>) { next unless $_ =~ m/\s+proc\d{1,}/; $num_proc++ }; close (LSCFG); return $num_proc }; sub total_memory { my $total_ram; open (LSATTR, "/usr/sbin/lsattr -El sys0|"); while (<LSATTR>) { next unless $_ =~ m/^realmem/; (undef, $total_ram) = split(/\s+/, $_); last }; close (LSATTR); $total_ram = ($total_ram / 1024); return $total_ram }; sub total_swap { my $total_swap; open (LSPS, "/usr/sbin/lsps -s|"); while (<LSPS>) { if ( $_ =~ m/\s+(\d{1,})MB/) { $total_swap = $1 }; }; close (LSPS); return $total_swap }; sub aix_version { my @ml; open (INSTFIX, "/usr/sbin/instfix -i|"); while (<INSTFIX>) { next unless /\s+All filesets for (.*)_AIX_ML/; my $ml = $1; $ml =~ s/\.//g; push (@ml, $ml); }; close (INSTFIX); my @ml_sorted = sort { $b cmp $a } @ml; return $ml_sorted[0] }; sub model_type { my $model = &amp;amp;hardware_info("model"); return $model }; sub processor_arch { my $arch = &amp;amp;hardware_info("arch"); return $arch }; sub processor_speed { my $speed = &amp;amp;hardware_info("speed"); return $speed } sub hardware_info { my $infowanted = shift || "ALL"; my ($model, $speed, $arch); my %rs6k = ( '02' => { model => "7015-930", speed => "25", arch => "Power" }, '10' => { model => "7013-530 or 7016-730", speed => "25", arch = +> "Power" }, '11' => { model => "7013-450", speed => "30", arch => "Power" }, '14' => { model => "7013-540", speed => "30", arch => "Power" }, '18' => { model => "7013-53H", speed => "33", arch => "Power" }, '1C' => { model => "7013-550", speed => "41.6", arch => "Power" } +, '20' => { model => "7015-930", speed => "25", arch => "Power" }, '2E' => { model => "7015-950", speed => "41", arch => "Power" }, '30' => { model => "7013-520", speed => "20", arch => "Power" }, '31' => { model => "7012-320", speed => "20", arch => "Power" }, '34' => { model => "7013-52H", speed => "25", arch => "Power" }, '35' => { model => "7012-32H", speed => "25", arch => "Power" }, '37' => { model => "7012-340", speed => "33", arch => "Power" }, '38' => { model => "7012-350", speed => "41", arch => "Power" }, '41' => { model => "7011-220", speed => "33", arch => "RSC" }, '43' => { model => "7008-M20 or 7008-M2A", speed => "33", arch => + "Power" }, '46' => { model => "7011-250", speed => "66", arch => "PowerPC" } +, '47' => { model => "7011-230", speed => "45", arch => "RSC" }, '48' => { model => "7009-C10", speed => "80", arch => "PowerPC" } +, '57' => { model => "7012-390 or 7030-3BT or 9076-SP2 Thin", speed +=> "67", arch => "Power2" }, '58' => { model => "7012-380 or 7030-3AT", speed => "59", arch => + "Power2" }, '59' => { model => "7012-39H or 9076-SP2 Thin", speed => "67", arc +h => "Power2" }, '5C' => { model => "7013-560", speed => "50", arch => "Power" }, '63' => { model => "7015-970 or 7015-97B", speed => "50", arch => + "Power" }, '64' => { model => "7015-980 or 7015-98B", speed => "62.5", arch +=> "Power" }, '66' => { model => "7013-580", speed => "62.5", arch => "Power" } +, '67' => { model => "7013-570 or 7015-R10", speed => "50", arch => + "Power" }, '70' => { model => "7013-590 or 9076-SP2 Wide", speed => "66", arc +h => "Power2" }, '71' => { model => "7013-58H", speed => "55", arch => "Power2" }, '72' => { model => "7013-59H or 7015-R20 or 9076-SP2 Wide", speed +=> "66", arch => "Power2" }, '75' => { model => "7012-370 or 7012-375 or 9076-SP1 Thin", speed +=> "62", arch => "Power" }, '76' => { model => "7012-360 or 7012-365", speed => "50", arch => + "Power" }, '77' => { model => "7012-350 or 7012-355 or 7013-55L", speed => "4 +1 or 41.6", arch => "Power" }, '79' => { model => "7013-591 or 9076-SP2 Wide", speed => "77", arc +h => "Power2" }, '80' => { model => "7015-990", speed => "71.5", arch => "Power2" +}, '81' => { model => "7015-R24", speed => "71.5", arch => "Power2" +}, '89' => { model => "7013-595 or 7076-SP2 Wide", speed => "135", ar +ch => "P2SC" }, '94' => { model => "7012-397 or 9076-SP2 Thin", speed => "160", ar +ch => "P2SC" }, 'A0' => { model => "7013-J30", speed => "75", arch => "PowerPC" } +, 'A1' => { model => "7015-J40", speed => "112", arch => "PowerPC" +}, 'F0' => { model => "7007-N40", speed => "50", arch => "ThinkPad" +}, ); my $modelid = substr(`/usr/bin/uname -m`, 8, 2); if (exists($rs6k{$modelid})) { ($model, $speed, $arch) = ($rs6k{$modelid}{model}, $rs6k{$modeli +d}{speed}, $rs6k{$modelid}{arch}) } elsif ($modelid eq "4C") { my $uname = `uname -M`; if ($uname =~ /S70/) { ($model, $speed, $arch) = ("7017-S70", "125", "RS64") } elsif ($uname =~ /S7A/) { ($model, $speed, $arch) = ("7017-S7A", "262", "RD64-II") } elsif ($uname =~ /S80/) { ($model, $speed, $arch) = ("7017-S80", "450", "RS-III") } elsif ($uname =~ /F40/) { ($model, $speed, $arch) = ("7025-F40", "160 or 233", "PowerPC") + } elsif ($uname =~ /H10/) { ($model, $speed, $arch) = ("7026-H10", "160 or 233", "PowerPC") + } elsif ($uname =~ /H70/) { ($model, $speed, $arch) = ("7026-H70", "340", "RS64-II") } elsif ($uname =~ /260/) { ($model, $speed, $arch) = ("7043-260", "200", "Power3") } elsif ($uname =~ /248/) { ($model, $speed, $arch) = ("7248-100 or -120 or -132", "100 or +120 or 132", "PowerPersonal") } elsif ($uname =~ /B50/) { ($model, $speed, $arch) = ("7046-B50", "375", "PowerPC") } elsif ($uname =~ /042|043/) { ($model, $speed, $arch) = ("7043-140 or -150 or -240", "166 or +200 or 233 or 332 or 375", "PowerPC") } elsif ($uname =~ /F50|H50|270/) { $model = "7025-F50 or 7026-H50 or 9076-SP Silver Node"; $arch = "PowerPC"; open (LSCFG, "/usr/sbin/lscfg -vp |"); while (<LSCFG>) { next unless (/ZC.*PS/); if (/PS=0009E4F580/i) { $speed = "166 MHz" } elsif (/PS=0013C9EB00/i) { $speed = "332 MHz" } else { $speed = "UNKN" } }; close (LSCFG) } else { ($model, $speed, $arch) = ("UNKN", "UNKN", "UNKN") }; } elsif ($modelid =~ /A3|A4|A6|A7/) { if ($modelid eq "A3") { $model = "7015-R30" } elsif ($modelid eq "A4") { $model = "7015-R40 or -R50 or 9076-SP2 + High" } elsif ($modelid eq "A6") { $model = "7012-G30" } elsif ($modelid eq "A7") { $model = "7012-G40" } else { $model = "UNKN" } $arch = "PowerPC"; open (LSCFG, "/usr/sbin/lscfg -vl cpucard0 |"); while (<LSCFG>) { next unless /\s+FRU/; if (/(E1D|C1D)/) { $speed = "75" ; last } elsif (/(C4D|E4D)/) { $speed = "112"; last } elsif (/(C4D|E4D)/) { $speed = "200"; last } else { $speed = "UNKN"} }; close (LSCFG) } elsif ($modelid =~ /C0|C4/) { if ($modelid eq "C0") { $model = "7024-E20 or -E30" } elsif ($modelid eq "C4") { $model = "7025-F30" } else { $model = "UNKN" } $arch = "PowerPC"; open (LSCFG, "/usr/bin/lscfg -vp |"); while (<LSCFG>) { next unless /.+\(ZA\)\.+PS=\d{1,}/; }; close (LSCFG); $speed = $1; } else { ($model, $speed, $arch) = ("UNKN", "UNKN", "UNKN") }; if ($infowanted eq "model") { return $model } elsif ($infowanted eq "speed") { return $speed } elsif ($infowanted eq "arch" ) { return $arch } elsif (defined(wantarray)) { return ($model, $speed, $arch) } }; 1; __END__ =head1 NAME Aix::Sysinfo - Perl extension for obtaining information about an AIX (RS/6000) system =head1 SYNOPSIS use Aix::Sysinfo; $hostname = hostname; $serialno = serial_number; $numprocs = processor_count; $totlram = total_memory; $totlswap = total_swap; $aixvers = aix_version; $sysmodel = model_type; $sysarch = processor_arch; $procspd = processor_speed; ($model, $arch, $speed) = hardware_info; =head1 DESCRIPTION =head2 This module provides a Perl interface for accessing information about an RS/6000 machine running the AIX operating system. =head1 FUNCTIONS =item hostname Returns a scalar containing the hostname of the system. The value is retrieved from the output of the "/usr/bin/hostname" command. =item serial_number Returns a scalar containing the unique ID number for the system. The value is derived from the output of the "/usr/bin/uname -m" command. =item processor_count Returns a scalar containing the number of processors in the system. T +he value is derived from the output of the "/usr/sbin/lscfg -vp" command. =item total_memory Returns a scalar containing the total amount of real memory in the system, in MB. The value is derived from the output of the "/usr/sbin/lsattr -El sys0" command. =item total_swap Returns a scalar containing the total amount of swap space in the system, in MB. The value is derived from the output of the "/usr/sbin/lsps -s" command. =item aix_version Returns a scalar containing the version of AIX and the latest complete maintenance level on ths system, in the form "VRMF-ML". The value is derived from the output of the "/usr/sbin/instfix -i" command. =item model_type Returns a scalar containg the hardware model type. See description of I<hardware_info> for more information. =item processor_arch Returns a scalar containing the processor architecture. See descripti +on of I<hardware_info> for more information. =item processor_speed Returns a scalar containing the speed of the system's processors, in MHz. See description of I<hardware_info> for more information. =item hardware_info Returns a three-element list containing the model type, processor architecture, and processor speed of the system, in that order. It is more efficient to use this function if you want all of the information returned, instead of the previous three, as those functions simply cal +l this one. The values returned by this function are obtained in various ways, as described in the IBM TechDoc C<Determing CPU Speed in AIX>, available from B<http://techsupport.services.ibm.com/rs6k/techbrowse/>. This article describes methods of getting the desired information. Unfortunately, though, some of the returned values are not C<definiative>. =head1 VERSION =head2 0.1 =head1 BUGS =head2 Since I do not have access to every combination of hardware and operating system, this module has been tested in a small subset of possible environments. Therefor, there may be "unexpected behieviour" in some of the environments where I have not been able to test. For that reason, I'd really apreciate it if people would e-mail me if any of the values produced by this module do not conform with their expectations. Also, because this is such an early release version, the names of functions and the types of values returned are subject to change, base +d on feedback from users. Don't depend on this module (yet!) for use in production code. =head1 TO-DO =item Find a more definitiave way of obtaining some of the information returned by functions in this module. =item Add an object-oriented interface. =item Add many more functions. =item Add features requested by AIX sysadmins (hint, hint!) =head1 AUTHOR Sandor W. Sklar <mailto:ssklar@stanford.edu> <http://whippet.stanford.edu/~ssklar/> =cut

Comment on please review my first module
Download Code
Re: please review my first module
by wog (Curate) on Jul 01, 2001 at 08:22 UTC
    I see a few minor problems. First require 5.6.0 is not a good idea. You probably want something like use 5.006 instead. The require should execute after the use warnings, not what you want. Also the ability to use 1.2.3 type version strings was not avialable till perl version 5.6.0. You might want to consider even trying to make your module work on perl 5.005.

    As for your hardware_info function, I think you could improve it by caching the hardware info, only computing it once, rather then recomputing the same info three times if someone calls model_type, processor_arch, and processor_speed in succession.

    I wonder if you really want to call your module AIX::SysInfo, or similar.

    As for your export list, it's not a good idea to export so much by default. Try to limit or eliminate your default export list. You also have declared an export tag which does nothing. Best to get rid of it if its going to do nothing. </p.

    Additionally, Sys::Hostname already exists and is standard. Your hostname function is not worth much.

    If you want to be paranoid, you should be checking the return values of the various programs you are calling after the close of their pipes, in case something goes horribly wrong, and maybe checking if something goes wrong with your backticks.

      Thanks for the hints! I've taken some of your advice, and I've got a few questions/comments on your other points.

      I've changed the module so that it now runs on several older versions; the oldest I've got installed anywhere is 5.001, and it's got a problem there, but its fine now on 003 and 005.

      I've also gotten rid of the "%EXPORT_TAGS" and "@EXPORT_OK" vars, as they were empty. I'm not clear on how I would reduce the number of items exported, since I want to provide access to the functions that I am exporting.

      I plan on keeping the 'hostname' function in there, cause I thought people would like to have that function available without having to load another module. It adds very little overhead (as far as I can tell.)

      Thanks for the suggestions! -s-
Re: please review my first module
by PetaMem (Priest) on Jul 01, 2001 at 14:49 UTC
    Hi,

    basically I would only like to talk about the name of the module.
    This seems more like a categorical problem with perl modules as
    there seems to be no clearer definition how to name a certain
    functionality. So why donīt you just name it Sysinfo::AIX?

    Is this issue discussed elsewhere in detail (naming conventions
    for perl modules)? In my opinion itīs desireable to have a maximum
    portable solution, so there could be modules like Sysinfo::Linux
    and so on, and someone could write a Sysinfo-Wrapper, which would then
    delegate queries to the OS-specific modules or just returning some
    kind of N/A.

    Ciao

      Thank you for your comment ... I can understand the point you are making, and it does seem to make sense.

      However, I was thinking that in the future, I (or someone) could write additional modules for AIX, along the lines of "AIX::DiskInfo", or "AIX::ODM", and so on.

      The issue you bring up is certainly valid, and I'd like to see discussion there.

      thanks, -s-
Re: please review my first module
by VSarkiss (Monsignor) on Jul 01, 2001 at 21:56 UTC

    This may entail a lot of work for you, but you asked ;-)

    I would suggest implementing the interface as a tied hash. As others have pointed out in this thread, you're exporting many sub names by default. Since all these values are logically related, it makes sense (to me, anyway) to collect them into a single data structure. That is, your caller would do something like this:

    use AIX::Sysinfo; # depending on your decision to rename it tie %sysinfo, 'AIX::Sysinfo'; my $aix_hostname = $sysinfo{hostname}; # for example my $aix_version = $sysinfo{aix_version}; # for example
    It may be less work than it appears; your module would only have to implement the "reading" functions of the hash, not the "writing" ones. Also, remember you can inherit from Tie::Hash to take care of a lot of the work.

    Another suggestion is to cache up the values from the various exernal calls. As it stands, every time I ask for the aix_version, for example, the code will fork and exec to retrieve the same value. But if you make some room in the modeul, you can save the returned values once and re-use them for the life of the program. (It's OK to save these values once since none of them can change without at least rebooting the machine.)

    Note that you can cache the values regardless of whether you use the tied-hash interface, but IMHO it would be nice to combine the two.

    HTH

    Update 7/2/01: Better link to man page courtesy of tye.

      thanks for the great suggestions. unfortunately, you've brought on *more* questions! :-)

      I think the idea of using a tied hash is very elegant; the problem is that, though I've read the manpage you've linked to, and also the Tied Variables chapter in my Camel-3rdEd book, my brain is in a total fog. Perhaps my problem is that I still don't really understand the object-oriented universe (no matter how hard I've tried!), and all of the examples in the manpage/book use OO syntax (which I've avoided.)

      Is it possible to implement the tied variable in my module without getting into the OO realm, or am I missing the point? (the point being that tied variables are fundamentally part of OO perl syntax.)

      Thanks again for the help; anything I can do to make this code better, I'm open to. Just not sure that my little brains is up for the task.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://92983]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others cooling their heels in the Monastery: (10)
As of 2014-10-20 18:30 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    For retirement, I am banking on:










    Results (88 votes), past polls