simplify config->from_file by not requiring full

file path. Instead, figure it out the same way as the 4 of the 5 caller do (by calling config_dir). Allow the 2nd arg to be the file name.

lib/Qpsmtpd.pm

@@ -6,7 +6,7 @@ our $VERSION = '0.95';
 use vars qw($TraceLevel $Spool_dir $Size_threshold);
 
 use lib 'lib';
-use base 'Qpsmtpd::Base';
+use parent 'Qpsmtpd::Base';
 use Qpsmtpd::Address;
 use Qpsmtpd::Config;
 use Qpsmtpd::Constants;
@@ -56,35 +56,24 @@ sub hooks {
 sub load_logging {
     my $self = shift;
 
-    # avoid triggering log activity
+    return if $LOGGING_LOADED;     # already done
-    return if ($LOGGING_LOADED || $hooks->{'logging'});
+    return if $hooks->{'logging'}; # avoid triggering log activity
 
-    my $configdir  = $self->config_dir("logging");
+    my @plugin_dirs = $self->conf->from_file('plugin_dirs');
-    my $configfile = "$configdir/logging";
+    if (!@plugin_dirs) {
-    my @loggers    = $self->conf->from_file($configfile, 'logging');
-
-    $configdir  = $self->config_dir('plugin_dirs');
-    $configfile = "$configdir/plugin_dirs";
-    my @plugin_dirs = $self->conf->from_file($configfile, 'plugin_dirs');
-    unless (@plugin_dirs) {
         my ($name) = ($0 =~ m!(.*?)/([^/]+)$!);
         @plugin_dirs = ("$name/plugins");
     }
 
-    my @loaded;
+    my @loggers = $self->conf->from_file('logging');
     for my $logger (@loggers) {
-        push @loaded, $self->_load_plugin($logger, @plugin_dirs);
+        $self->_load_plugin($logger, @plugin_dirs);
-    }
-
-    foreach my $logger (@loaded) {
         $self->log(LOGINFO, "Loaded $logger");
     }
 
-    $configdir  = $self->config_dir("loglevel");
+    $TraceLevel = $self->conf->from_file('loglevel');
-    $configfile = "$configdir/loglevel";
-    $TraceLevel = $self->conf->from_file($configfile, 'loglevel');
 
-    unless (defined($TraceLevel) and $TraceLevel =~ /^\d+$/) {
+    unless (defined($TraceLevel) && $TraceLevel =~ /^\d+$/) {
         $TraceLevel = LOGWARN;    # Default if no loglevel file found.
     }
 

lib/Qpsmtpd/Config.pm

@@ -90,38 +90,35 @@ sub default {
 sub get_qmail {
     my ($self, $config, $type) = @_;
     $self->log(LOGDEBUG, "trying to get config for $config");
-    my $configdir = $self->config_dir($config);
-
-    my $configfile = "$configdir/$config";
 
     # CDB config support really should be moved to a plugin
     if ($type and $type eq "map") {
-        return $self->get_qmail_map($config, $configfile);
+        return $self->get_qmail_map($config);
     }
 
-    return $self->from_file($configfile, $config);
+    return $self->from_file($config);
 }
 
 sub get_qmail_map {
-    my ($self, $config, $configfile) = @_;
+    my ($self, $config, $file) = @_;
 
-    if (!-e $configfile . ".cdb") {
+    $file ||= $self->config_dir($config) . "/$config.cdb";
-        $self->log(LOGDEBUG, "File $configfile.cdb does not exist");
+
+    if (!-e $file) {
+        $self->log(LOGDEBUG, "File $file does not exist");
         $config_cache{$config} ||= [];
         return +{};
     }
     eval { require CDB_File };
 
     if ($@) {
-        $self->log(LOGERROR,
+        $self->log(LOGERROR, "No CDB Support! Did NOT read $file, could not load CDB_File: $@");
-"No CDB Support! Did NOT read $configfile.cdb, could not load CDB_File module: $@"
-        );
         return +{};
     }
 
     my %h;
-    unless (tie(%h, 'CDB_File', "$configfile.cdb")) {
+    unless (tie(%h, 'CDB_File', $file)) {
-        $self->log(LOGERROR, "tie of $configfile.cdb failed: $!");
+        $self->log(LOGERROR, "tie of $file failed: $!");
         return +{};
     }
 
@@ -132,17 +129,19 @@ sub get_qmail_map {
 }
 
 sub from_file {
-    my ($self, $configfile, $config, $visited) = @_;
+    my ($self, $config, $file, $visited) = @_;
-    if (!-e $configfile) {
+    $file ||= $self->config_dir($config) . "/$config";
+
+    if (!-e $file) {
         $config_cache{$config} ||= [];
         return;
     }
 
     $visited ||= [];
-    push @$visited, $configfile;
+    push @$visited, $file;
 
-    open my $CF, '<', $configfile or do {
+    open my $CF, '<', $file or do {
-        warn "$$ could not open configfile $configfile: $!";
+        warn "$$ could not open configfile $file: $!";
         return;
     };
     my @config = <$CF>;
@@ -180,8 +179,8 @@ sub from_file {
             }
             push @{$visited}, $inclusion;
 
-            for my $inc ($self->expand_inclusion($inclusion, $configfile)) {
+            for my $inc ($self->expand_inclusion($inclusion, $file)) {
-                my @insertion = $self->from_file($inc, $config, $visited);
+                my @insertion = $self->from_file($config, $inc, $visited);
                 splice @config, $pos, 0, @insertion;    # insert the inclusion
                 $pos += @insertion;
             }

t/qpsmtpd-config.t

@@ -94,7 +94,7 @@ sub __get_qmail {
 sub __get_qmail_map {
     eval "require CDB_File";   ## no critic (StringyEval)
     if (!$@) {
-        my $r = $config->get_qmail_map('users', 't/config/users');
+        my $r = $config->get_qmail_map('users', 't/config/users.cdb');
         ok(keys %$r, 'get_qmail_map("users.cdb")');
         ok($r->{'!example.com-'}, "get_qmail_map, known entry");
     };
@@ -102,7 +102,7 @@ sub __get_qmail_map {
 
 sub __from_file {
     my $test_file = 't/config/test_config_file';
-    my @r = $config->from_file($test_file, 'test_config_file');
+    my @r = $config->from_file('test_config_file', $test_file);
     ok( @r, "from_file, $test_file");
     cmp_ok('1st line with content', 'eq', $r[0], "from_file string compare");
     ok( !$r[1], "from_file");