Guy Hulbert wrote:
On Thu, 2008-02-14 at 10:43 -0500, Chris Lewis wrote:
BUT, here's the rub: even if you try to work around that by setting
config/me to the FQDN, config(me) still returns `hostname`. This
implies to me a bug in qpsmtpd's config handling.
I think 'bug' is a bit strong.
No, it's definately a bug. Setting config/me _should_ override what
qpsmtpd's config(me) returns - the fact that hostname is called is
irrelevant.
I've investigated, and found the problem in Qpsmtpd.pm
When you call config(something), Qpsmtpd.pm's "sub config" is called.
The first thing it does is check $_config_cache{}, and if there's
something there, it returns that. Otherwise, it falls through to
get_qmail_config to get the value. Unless you're trying to use CDB
support, get_qmail_config ultimately calls _config_from_file, which sets
_config_cache{} as you'd expect.
[In get_qmail_config there's a question just before it returns what it
found from the CDB as to whether it should be cached. It ain't. It
probably should be.]
With me so far?
clear_config_cache() is used to initialize _config_cache{}. The bug is
that clear_config_cache then initializes _config_cache{me} and
_config_cache{timeout) to `hostname` and "1200" respectively (from the
defaults{} hash near the beginning).
Code (current SVN head I think):
sub clear_config_cache {
$_config_cache = {};
for (keys %defaults) {
$_config_cache->{$_} = [$defaults{$_}];
}
}
In other words, these aren't "defaults", they're hardcoded and there's
no way to override. This is not right.
Replacing the for loop thusly:
for (keys %defaults) {
$self->config($_); # called to set _config_cache
$_config_cache->{$_} ||= [$defaults{$_}];
}
Should work by using the defaults only if config() couldn't find an
alternative value via Qmail or Qpsmtpd config file. Except if it's
found by CDB and not cached. That's really not right either, but at
least it'd work for me. You could fix that by caching the CDB results
too - I make no comment as to whether that's the right thing to do.
The config() code should be rewritten to be a bit more straight-forward
- it looks fairly clear to me that _config_cache{} was an afterthought,
because config() does default handling _too_ (but because of
clear_config_cache() that part is dead code), but if you simply remove
the for loop from clear_config_cache, letting config() handle defaults,
the defaults don't get cached, and things slow down.
There is a project (sipx - see sipfoundry.org) using 'hostname -s' and
'hostname -f' on linux and the software has been ported to HP-UX, which
is also SYSVr4 like solaris. It probably runs on solaris also.
However, they just use 'hostname' to _create_ their configuration files.
Frankly, I don't care that there are other versions of hostname or I
could zap hostname to the FQDN on Solaris. You can't do that in
distributed software. Making qpsmtpd work properly at the expense of
breaking the rest of the OS (eg: NIS) is not a viable option.
What is really missing from qpsmtpd IMO is a configuration tool. I'm
interested in working on one but I won't be getting back to qpsmtpd
until March at the earliest.
I'd be happy with $self->qp->config() working as expected.
Steve has walked tested the code in 0.40, and it works right there.
0.42rc1 (SVN head) doesn't.
