views:

326

answers:

3

I am cleaning my Perl code for production release and came across a weird warning in the Apache error log.

It says:

[Thu Nov 5 15:19:02 2009] Clouds.pm: Use of uninitialized value $name in substitution (s///) at /home/mike/workspace/olefa/mod-bin/OSA/Clouds.pm line 404.

The relevant code is here:

my $name         = shift @_;
my $name_options = shift @_;

$name_options = $name_options eq 'unique'     ? 'u'
              : $name_options eq 'overwrite'  ? 'o'
              : $name_options eq 'enumerate'  ? 'e'
              : $name_options =~ m/^(?:u|o|e)$/ ? $name_options
              : q();

if ($name_options ne 'e') {
   $name =~ s/ /_/g;
}

So, why the warning of an uninitialized variable as it is clearly initialized?

+11  A: 

The warning simply means that $name was never filled with a value, and you tried doing a substitution operation (s///) on it. The default value of a variable is undefined (undef).

Looking back through your script, $name gets its value from @_. This means either @_ was empty, or had its first value as undef.

clintp
A: 

Debug by divide and conquer

It is quite usual to run into bugs that are "obviously impossible" --- at the first glance. I usually try to confirm my assumptions with simple print statements (or equivalent ways to get some information back from the program: For CGI scripts, a simple print can ruin your headers).

So, I would put a statement like

print "testing: ", defined($name)? "defined: '$name'" : "undef", "\n";

into the code at the suspected line. You might be surprised about the possible output options:

  • "testing: undef" --- this means that your function was called with an undefined first argument. Unlikely but possible. You may want to use caller() to find out from where it was called and check the data there.
  • no output at all! Maybe you look at the wrong source file or at the wrong line. (I don't suspect that's the case here, but it does happen to me).
  • "testing: defined: 'some data'" --- oops. ask a question on stackoverflow.
Yaakov Belch
It's easier to use Carp for this because it points out who called you in the offending case. You also probably want to print to STDERR so it shows up in the error log, which is easier to grep. :)
brian d foy
Or: `print "testing: ", Data::Dumper->new([$name],['name'])->Useqq(1)->Dump;`
ysth
+2  A: 

Depending on what your subroutine needs to do, validate your values before you use them. In this case, since you need something in $name, croak if there isn't something in that variable. You'll get an error message from the perspective of the caller and you'll find the culprit.

Also, you can lose the complexity of the conditional operator chain by making it a hash lookup, which also gives you a chance to initialize $name_option. In your fallback case, you leave $name_option undefined:

use 5.010;

use Carp;

BEGIN {
my %valid_name_options = map {
 $_
 substr( $_, 0, 1 ),
 } qw( unique overwrite enumerate );

some_sub {
 my( $name, $name_options ) = @_;

 croak( "Name is not defined!" ) unless defined $name;
 $name_options = $valid_name_options{$name_options} // '';

 if ($name_options ne 'e') {
  $name =~ s/ /_/g;
  }

 ...
 }
}
brian d foy