Another non-standard search/sort field :)

Bron Gondwana brong at fastmail.fm
Fri Sep 11 07:49:11 EDT 2015


On Fri, Sep 11, 2015, at 20:00, Vladislav Bogdanov wrote:
> 11.09.2015 12:01, Bron Gondwana wrote:
> > On Fri, Sep 11, 2015, at 16:48, Vladislav Bogdanov wrote:
> >> 11.09.2015 03:52, Bron Gondwana wrote:
> >>> sort: spamscore search: spamabove / spambelow
> >>>
> >>> These use the X-Spam-score header which is a floating point number
> >>> with a single decimal place usually, i.e. 5.0, 17.3.  spamabove is GE
> >>> and spambelow is LT.
> >>>
> >>> I'm going to push this back, because it doesn't clash with anything.
> >>> It's kinda nice to be able to sort by spamscore to quickly put the
> >>> focus on the most likely to be be wrongly classified messages, and
> >>> we're going to support that in our interface at some stage.
> >>>
> >>> Bron.
> >>>
> >>
> >> Ah, I have a nice patch for spamtest extension against 2.4.17.
> >>
> >> It connects to spamd itself from lmtpd, checks the message and sets
> >> additional headers. Sieve integration is done too.
> >>
> >> Need to send it here.
> >
> > That would be great.
> 
> Attached. Just found that it inconsistently uses tabs/spaces. I hope 
> that is not an issue at least for initial review.

Initial impressions:

I would rename 's' to 'fd'.  Everyone knows what 'fd' does, s could be anything,
and being an int, it's totally un-typesafe.

I'm kind of tempted to suggest using the prot.c prot_write, prot_read functions
rather than raw fwrite and friends, but it's a real nit - they look fine.  THOUGH,
I would test for errno if you're going to use them.  Running in a real process,
you could signals.  The prot stuff hides that for you.

I guess the nice thing is, it will just fail back to the MTA which will try again later,
so no real damage gets done.

I've got a sneaking feeling that your entire spamtest_parse_hosts could be
turned into a tight little piece of code based on strarray_split() - but it looks fine.

All the code looks like it works (which is not a surprise, because it's been used).
My main concerns would be around signal safety in the file IO syscalls.

Thanks so much for posting it :)  I'll get Ellie to have a look at it (yay minions)
and integrate it.

Cheers,

Bron.

-- 
  Bron Gondwana
  brong at fastmail.fm


More information about the Cyrus-devel mailing list