Hanno Hecker wrote:
On Sun, 30 Mar 2008 21:51:26 -0700
Ask Bjørn Hansen <[EMAIL PROTECTED]> wrote:

On Mar 30, 2008, at 4:39 PM, Markus Ullmann wrote:

After tracing it down to rev 814-818, a friend of mine came up with this patch and it seems to make it work for me again. Maybe someone with deep qpsmtpd knowledge could take a look at it

I think the patch breaks a feature of being able to change the hooks per Qpsmtpd instance ... Is that right?
Don't think so. I'd prefer to have them "exported" like in the attached
diff. Someone test if this works with the tls plugin?

Yeah, well, if you're going to do *that*, you should probably make that a getter/setter method, so you expose a way to change the list per object (to answer Ask's question too). The use of the $Qpsmtpd::hooks package global is clearly not what we want.

I haven't had a change to review the changes that broke this feature (my daughter's Bat Mitzvah was this weekend and I'm only now able to regain some form of limited consciousness). Just a sec...hmmm, it seems this chunk here (from c814) is nominally responsible for the breakage:

--- lib/Qpsmtpd.pm      (revision 813)
+++ lib/Qpsmtpd.pm      (revision 814)
@@ -18,7 +18,7 @@
   # need to do this differently that other plugins so as to
   # not trigger logging activity
   my $self = shift;
-  return if $self->{hooks}->{"logging"};
+  return if $hooks->{"logging"};
   my $configdir = $self->config_dir("logging");
   my $configfile = "$configdir/logging";
   my @loggers = $self->_config_from_file($configfile,'logging');

but I don't know what Matt was trying to fix. As Ask surmised, the whole point was to have the object hooks, not a package hooks, be used, since the hooks registered are attributes of the object, not the class.

Attached is a diff that fixes both tls and auth, by backing out 814 and tweaking a couple of other lines. I won't commit it until Matt pipes up why he tried to make hooks a package global...

John

Index: lib/Qpsmtpd.pm
===================================================================
--- lib/Qpsmtpd.pm	(revision 872)
+++ lib/Qpsmtpd.pm	(working copy)
@@ -9,7 +9,6 @@
 
 $VERSION = "0.43rc1";
 
-my $hooks = {};
 my %defaults = (
 		  me      => hostname,
 		  timeout => 1200,
@@ -34,7 +33,7 @@
   # not trigger logging activity
   return if $LOGGING_LOADED;
   my $self = shift;
-  return if $hooks->{"logging"};
+  return if $self->{hooks}->{"logging"};
   my $configdir = $self->config_dir("logging");
   my $configfile = "$configdir/logging";
   my @loggers = $self->_config_from_file($configfile,'logging');
@@ -290,10 +289,13 @@
 sub load_plugins {
   my $self = shift;
   
+  $self->log(LOGWARN, "Plugins already loaded") if $self->{hooks};
+  $self->{hooks} = {};
+  
   my @plugins = $self->config('plugins');
   my @loaded;
 
-  if ($hooks->{queue}) {
+  if ($self->{hooks}->{queue}) {
     #$self->log(LOGWARN, "Plugins already loaded");
     return @plugins;
   }
@@ -374,6 +376,7 @@
 
 sub run_hooks {
   my ($self, $hook) = (shift, shift);
+  my $hooks = $self->{hooks};
   if ($hooks->{$hook}) {
     my @r;
     my @local_hooks = @{$hooks->{$hook}};
@@ -385,6 +388,7 @@
 
 sub run_hooks_no_respond {
     my ($self, $hook) = (shift, shift);
+    my $hooks = $self->{hooks};
     if ($hooks->{$hook}) {
         my @r;
         for my $code (@{$hooks->{$hook}}) {
@@ -485,6 +489,7 @@
   my $self = shift;
   my ($hook, $code, $unshift) = @_;
 
+  my $hooks = $self->{hooks};
   if ($unshift) {
     unshift @{$hooks->{$hook}}, $code;
   }

Reply via email to