Hello Everyone,
I occasionally like to download perl programs off the web to see how they work. And if there are any bugs I try to report back to the original author.
This code was found verbatim (except for the file name) in two different files.
my $conf = which_conf($configuration_file); if (-f $conf) { my @conf_args; open(my $conffile, '<', "$conf") or warn "$P: Can't find a readable $configuration_file file $! +\n"; while (<$conffile>) { my $line = $_; $line =~ s/\s*\n?$//g; $line =~ s/^\s*//g; $line =~ s/\s+/ /g; next if ($line =~ m/^\s*#/); next if ($line =~ m/^\s*$/); my @words = split(" ", $line); foreach my $word (@words) { last if ($word =~ m/^#/); push (@conf_args, $word); } } close($conffile); unshift(@ARGV, @conf_args) if @conf_args; }
my $conf = which_conf($configuration_file); if (-f $conf) { my @conf_args; open(my $conffile, '<', "$conf") or warn "$P: Can't find a readable $configuration_file file $! +\n";
It looks like there is a race condition between the file test and the file open.
The file test does not print a message if the file is not found but the file open does.
If the file cannot be opened a message is printed and then another message is printed when readline tries to read from an invalid file handle.
The file name is copied to a string.
while (<$conffile>) { my $line = $_;
Data is assigned to $_ and then copied to $line but $_ is never used inside the loop.
$line =~ s/\s*\n?$//g;
This is the line that gobsmacked me. I ran some tests with Benchmark and re 'debug' compared to the FAQ answer "s/\s+$//;" and it turns out that this is a good example of what not to do.
This substitution was found in five places in one file and two places in the other file.
$line =~ s/^\s*//g;
Not as bad as the previous line but still not as good as the FAQ answer.
$line =~ s/\s+/ /g;
Correct code, nothing to see here.
next if ($line =~ m/^\s*#/); next if ($line =~ m/^\s*$/);
Why check for whitespace after you've just removed it?
my @words = split(" ", $line); foreach my $word (@words) { last if ($word =~ m/^#/); push (@conf_args, $word); }
This code is where the work is done and obviates the need for the previous five lines. split(" ", $line) removes all whitespace. foreach will not loop if @words is empty. $word =~ m/^#/ will skip to the next line if a '#' comment is found.
close($conffile);
Correct code, nothing to see here.
unshift(@ARGV, @conf_args) if @conf_args;
The conditional is not really required.
Where I found this code:
In reply to Critique of some perl code. by jwkrahn
For: | Use: | ||
& | & | ||
< | < | ||
> | > | ||
[ | [ | ||
] | ] |