Tomte has asked for the wisdom of the Perl Monks concerning the following question:

fellow monks,

I just finished another of our custom nagios checks, which I all write after a similar fashion. I would like you to comment on this from a perspective of style vs. practicability.

This is the skeleton or boilerplate I normaly use:

#!/usr/bin/perl -T # creator : Tom Regner <mail@address.tld> # created : Wed 11 Jun 2003 05:10:32 PM CEST # version : $Id: check_ifconfig,v 1.1 2003/06/12 13:17:35 tom Exp $ # copyright : (c) 2003 my company # use strict; use warnings; use Getopt::Long; use vars qw($opt_V $opt_h @opt_i @opt_n $PROGNAME $VERSION); use lib "/data/programme/nagios/libexec" ; use utils qw(%ERRORS &print_revision &support &usage &exit_); $PROGNAME = "progname"; #$VERSION = '$Revision'; # as originally posted $VERSION = '$Revision$'; # Update sub print_help (); sub print_usage (); $ENV{'PATH'}=''; $ENV{'BASH_ENV'}=''; $ENV{'ENV'}=''; Getopt::Long::Configure('bundling'); GetOptions ( "V" => \$opt_V, "version" => \$opt_V, "h" => \$opt_h, "help" => \$opt_h, ); if ($opt_V) { print_revision($PROGNAME,$VERSION); exit($ERRORS{'OK'}) ; } if ($opt_h) {print_help(); exit($ERRORS{'OK'});} # # main-test # # untaint args # check correctness of args my $msg = ''; # return message my $status = $ERRORS{OK}; # exit status # # perform test # exit_($status, $msg); # # subs for test-parts, recurring actions during the test, etc. # sub check { my ($stuff1, $stuff2,) = @_; my ($result, $dmsg) = (1, ""); # # do test, in case of error, change global variable $status # # assamble info into $dmsg # $msg .= $dmsg; # $msg is 'global' return $result; } # # nagios plugin-standards # sub print_usage () { } sub print_help () { print_revision($PROGNAME,$VERSION); print_usage(); support(); }

The test-logic is under 30 lines of code (including the subs) most of the time.

Is this bad style in your opinion? What would you change? How do you write your nagios-checks?

kind regards,
tomte


Hlade's Law:

If you have a difficult task, give it to a lazy person --
they will find an easier way to do it.

Update per author's request - dvergin 2003-06-13

Replies are listed 'Best First'.
Re: 'global' variables in small scripts -- a question of style?
by ctilmes (Vicar) on Jun 12, 2003 at 14:47 UTC
    Add a POD template, use Pod::Usage and get rid of print_usage() and use pod2usage().

      ++ctilmes,

      I wish it would be that easy ;), I would have to change utils.pm for that, and that means every check-script I've written. The way this is handled is given to you via the plugins-'framework' of nagios, so I just used it in the beginning of my nagios endeavour.

      It's something I hopefully accomplish before I leave here, to leave behind yet better maintainable code ;-)

      regards,
      tomte


      Hlade's Law:

      If you have a difficult task, give it to a lazy person --
      they will find an easier way to do it.

(jeffa) Re: 'global' variables in small scripts -- a question of style?
by jeffa (Bishop) on Jun 12, 2003 at 14:48 UTC
    Sometimes globals win out. Your code seems fine to me, but this line:
    $VERSION = '$Revision';
    should probably have a comment explaining that (even though you and i know it) $Revision is a CVS variable, and the single quotes are indeed intentional. Otherwise, it just looks like an error to someone who may not be experienced with CVS.

    jeffa

    L-LL-L--L-LL-L--L-LL-L--
    -R--R-RR-R--R-RR-R--R-RR
    B--B--B--B--B--B--B--B--
    H---H---H---H---H---H---
    (the triplet paradiddle with high-hat)
    

      Leading to my first editor-request ;-), the line should read

      $VERSION = '$Revision$;

      I will never have a thorough enough review by preview it seems ;-)

      regards,
      tomte


      Hlade's Law:

      If you have a difficult task, give it to a lazy person --
      they will find an easier way to do it.

Re: 'global' variables in small scripts -- a question of style?
by diotalevi (Canon) on Jun 12, 2003 at 20:49 UTC

    For things this small I often don't even use strict and never use lexicals unless I know I need scoping for something. Of course if it turns out that it doesn't immediately work right then I put all the goodies into place and do it the right way which for me means warnings are fatal as well. Usually the warnings show what I was doing wrong, the strict just forces me to be pedantic.

    use strict; use warnings FATAL => 'all';