What causes the warning "Use of uninitialized value" in my program?

Question

This one makes no sense to me. I have these two subroutines.

sub load_config_file {
    if (@_ eq '') {
        die RED . "No configuration file defined" . RESET . "\n";
    } else {
        if (! -e "@_") {
            die RED . "@_ not found!" . RESET . "\n";
        } else {
            if (`cat @_` eq '') {
                die RED . "$config_file_path is an empty file!" . RESET . "\n\n";
            } else {
                print "Configuration file:" . GREEN . "@_" . RESET . "\n";
                my $xml_obj = XML::Simple->new();
                my $config_xml = $xml_obj->XMLin("@_", SuppressEmpty => 1);
                %config_file = %$config_xml;
            }
        }
    }
} # End load_config_file

sub load_guest_os_file {
    if (@_ eq '') {
        die RED . "No guest operating system file defined" . RESET . "\n";
    } else {
        if (! -e "@_") {
            die RED . "@_ not found!" . RESET . "\n";
        } else {
            if (`cat @_` eq '') {
                die RED . "@_ is an empty file!" . RESET . "\n\n";
           } else {
                print "Guest OS file:" . GREEN . "@_" . RESET . "\n";
                my $xml_obj = XML::Simple->new();
                my $guest_os_xml = $xml_obj->XMLin("@_", SuppressEmpty => 1);
                %guest_os_file = %$guest_os_xml;
            }
        }
    }
} # End load_guest_os_file

Their purpose is to load a specific config file needed for my script. The first one, load_config_file, works perfect. But when I move onto the second one, load_guest_os_file, I get these errors from Perl:

Use of uninitialized value $_[0] in join or string at analyze.pl line 146.
Use of uninitialized value $_[0] in join or string at analyze.pl line 148.

Line 146 in my script is

if (! -e "@_") {

and line 148 is

die RED . "@_ not found!" . RESET . "\n";

What am I missing? When I call the subroutine thus:

load_config_file($config_file_path)
load_guest_os_file($guest_os_file_path)

… the values assigned to those two variables are

my $config_file_path = './config.xml'

and

my $guest_os_file_path = './guest_os.xml'

---

Edit: I should also add the values for the two variables coming from the command line arguments processed by Getopt::Long. If no value is assigned, the variable is just "declared", I think that's the term. I do not assign a value to it, it's just my $config_file_path; and my $guest_os_file_path;.

Update

Here is the code from the beginning of the script.

#!/usr/bin/perl
use strict;
use warnings;

# Modules to load
use Getopt::Long;
use Term::ANSIColor qw(:constants);
use XML::Simple;
use Net::Ping;
use Net::OpenSSH;
use Data::Dumper;

# Script version
my $version = 'v0.6';

my (%config_file, %guest_os_file, %machines_xml, $ssh_obj);

my @selected_mode;

# Configuration file
my $config_file_path;

# Guest OS file
my $guest_os_file_path;

# Exclusion file
my $exclude_file_path;

# Disables snapshot capture
my $no_snapshots = 0;

my $logfile_path;

my $verbose = 0;

# Program modes
my %program_modes = (
    analyze => \&analyze,
    backup  => \&backup,
    restore  => \&restore,
    help  => \&help,
);

GetOptions(
    'c=s' => \$config_file_path,
    'e=s' => \$exclude_file_path,
    'g=s' => \$guest_os_file_path,
    'l=s' => \$logfile_path,
    'v' => \$verbose,
    'x' => \$no_snapshots,
    'a' => sub { push @selected_mode, "analyze" },
    'b' => sub { push @selected_mode, "backup" },
    'h' => sub { push @selected_mode, "help" },
    'r' => sub { push @selected_mode, "restore" },
    's' => sub { push @selected_mode, "setup" },
);

# Show the help menu if no program mode has been selected
if (@selected_mode == 0) {

    help();

# Throw an error and show the help menu if too many modes are selected
} elsif (@selected_mode > 1) {

    print RED . "Too many program modes specified" . RESET . "\n";

    print "See help menu [-h] for further information\n";

# Run the selected program mode
} elsif (@selected_mode == 1) {

    if ($selected_mode[0] eq 'help') {

        help();

    } else {

        # Die unless user is root
        die RED . "You must be have superuser permissions to run this script" . RESET . "\n" unless ($> == 0);

        system "clear";

        print "Solignis's VMware $selected_mode[0] script $version for ESX\\ESX(i) 4.0+\n";

        load_config_file($config_file_path);

        if ($selected_mode[0] eq 'analyze') {

            load_guest_os_file($guest_os_file_path);

        } else {

            ######

        }

    }

}

Answer 1

In order to get the warnings mentioned above, the first parameter to subroutine load_guest_os_file has to be undefined (which is the default value after declaration).

From the source code you have shown, the only possibility I can see for this scenario to happen is that no valid option -g<path> was given, and so variable $guest_os_file_path is never assigned a value. Then subroutine load_guest_os_file would be called with an undefined value as its parameter like this

load_guest_os_file(undef)

and Perl would give these warnings.

Answer 2

If you use the subs with only one value, you might as well copy that over to a variable, instead of using @_, like so:

sub load_guest_os_file {
    my $path = shift;

The tests you are performing can be done better, and they do not need to be inside each other, since the only result is die:

$path    || die RED . "No guest operating system file defined" . RESET . "\n";
-e $path || die RED . "$path not found!" . RESET . "\n";
-s $path || die RED . "$path is an empty file!" . RESET . "\n\n";

The -e check is not functionally necessary, as -s will fail also if the file is missing. It will give a better error, though.

Also, if you are using arguments to your function, it might be more consistent to not manipulate global variables with the sub, and instead give a return value, such as:

    ...
    return %$config_xml;
}

%config_file = load_config_file($config_file_path);

Answer 3

Some genereal pointers on your code:

• Consider using elsif instead of the ever nesting else blocks.
• If you have a bunch of error conditions you're filtering out, consider using statement modifier if/unless logic.
• Consider using -z or -s to get your file size ( see http://perldoc.perl.org/functions/-X.html ).
• Unpack @_ at the top of your subroutines.
• Minimize use of global variables. Explicitly pass all data in and out of your subs.

Here's a cleaned up version of your first sub:

sub load_config_file {
    my $config_file = shift;

    die RED . "No configuration file defined" . RESET . "\n"
        unless defined $config_file;

    die RED . "$config_file not found!" . RESET . "\n"
        unless -e $config_file;

    die RED . "$config_file_path is an empty file!" . RESET . "\n\n"
        if -z $config_file;


    print "Configuration file:" . GREEN . "@_" . RESET . "\n";

    my $xml_obj = XML::Simple->new();
    my $config_xml = $xml_obj->XMLin("@_", SuppressEmpty => 1);

    return $config_xml;

} # End load_config_file

BTW, I am not sure what you have going on with the REDs and RESETs in your die messages, but I have a feeling that it could be better achieved with an exception handler.

Answer 4

This will always be false:

if (@_ eq '') {

When empty, the array gives 0 in scalar context, not ''. Just:

if ( ! @_ ) {

is sufficient to test if there was nothing passed.

But I think you actually mean to insure a defined value was passed:

if ( ! defined $_[0] ) {

To know why it $_[0] is undefined, we'd have to see the code from the declaration to where it is passed to the sub.