Re: [evolution-patches] Re: New hook for junk plugin (improved)

[Not Zed <notzed ximian com>]

Style has a few niggles.  K&r style braces!!

All of the checks in the junk plugin methods for a NULL junk plugin
pointer - i dont like those.  It shouldn't be being called if it isn't
set.  None of the junk processing should occur if there is no plugin
assigned.

There's absolutely no reason to call junk_plugin_init in the hook loader
- you know it does nothing because all its doing is calling your null
init callback!

I think you should return NULL for the name if the plugin is disabled.
Or find another string like "None" which already exists and use that,
the extra string to translate is redundant.

The target shouldn't have the item in it.  If the junk callbacks need
the item - which they dont anyway, it only contains internal data to the
hook that the plugin has no use of - that, and the target should be
wrapped in a hookdata struct.  I think you can just pass the target
directly with just the message in it, at least.

You dont need to check pointers for NULL when calling g_free, it already
adds that overhead for you.

The code shouldn't assign the junk plugin to the session - THEN validate
if it was ok - and free the plugin from under the session if it wasn't!

It doesn't handle having multiple junk plugins very well.  i.e. it just
overwrites the last one with a new one, leaking.

I'm not sure the best way here.  One is to just setup a single hook to
the session, but load all plugins into a list, and just run the first
enabled plugin for the callbacks, or just spit a warning if you already
have one loaded and another one comes along.  It has to do something
better than just leaking at least.


On Mon, 2005-07-04 at 16:26 +0530, Vivek Jain wrote:
> hi,
> 
> I am attaching the modified "em-junk-hook.c" and  patches to
> mail-session.c and camel-junk-plugin.c
> 
> Now I am assigning NULL to junk_plugin member of session while init.
> The plugin is supplied in the "construct" of the "item". If "item" is
> not constructed, there is no plugin to load.
> 
> file camel-junk-plugin.c is modified to handle the cases when there is
> no plugin at all.
> Please let me know if it looks ok.
> 
> Thanks,
> Vivek Jain
> 
> 
> 
> On Fri, 2005-06-24 at 14:33 +0530, Vivek Jain wrote:
> > Hi,
> > 
> > Now I am passing the CamelJunkPlugin object to the callbacks and
> > subclassing it into the EMJunkHookItem, so that I can get the "item"
> > when methods are invoked.
> > 
> > The camel/camel-junk-plugin.[ch] files have to be patched for this.
> > I have attached the improved em-junk-hook.[ch] files and a patch to
> > camel.
> > 
> > Please let me know if this looks ok.
> > 
> > Thanks,
> > Vivek Jain
> > 
> > 
> > On Thu, 2005-06-23 at 21:11 +0530, Vivek Jain wrote:
> > > hi,
> > > 
> > > I have created a hook that wraps up camelJunkPlugin and allows its
> > > methods to be exposed to the plugins. Using this hook one can write a
> > > plugin to use its own method for spam handling. As an example I have
> > > moved the spamassasin junk mail handling to a plugin which used this new
> > > hook.
> > > 
> > > I have added two files em-format-hook.c em-format-hook.h in the /mail
> > > and removed em-junk-filter.[ch] from here. I have moved em-junk-filter.c
> > > to the newly written sa-junk-plugin.
> > > 
> > > I am attaching the following:
> > > 
> > > 1. Patch for mail/
> > > 2. two new files
> > > 3. sa-junk-plugin
> > > 
> > > 
> > > Thanks,
> > > Vivek Jain
> > > 
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches