11.09.2015 14:49, Bron Gondwana wrote:
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.
It was initially written against newly-released 2.3.8. That time socket
fd was usually named 's' or 'sock'. 's' is shorter ;) Did that change?
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.
Yep, that never resulted in lost messages for last N+1 years in quite
busy setups (not as fastmail, but anyways...).
So, yes, that "works for me" and such change would be really cosmetic.
I just ported it to 2.4.17 recently without even looking much at the
code (but yes, that is tested on newer setups right from the patch date).
I can send previous revision (against 2.3.13) as well.
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.
No opinion. It was not available in 2.3.x. IMHO spamtest_parse_hosts is
very straight-forward.
And, that _may_ conflict with the line in TODO list (which I probably
will never do anyways because I have no idea how to make that fair
enough without initial lookups):
* Make load-balancing work if hostname that resolve to multiple A
records is used in "spamtest_spamd_hosts".
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.
Feel free to convert them to prot ones, but I do not feel it is strictly
required.
Thanks so much for posting it :) I'll get Ellie to have a look at it (yay
minions)
and integrate it.
Cheers,
Bron.
Best,
Vladislav
