Latest AV code

[Jeremy Allison]

On Fri, Dec 22, 2017 at 03:47:02PM +0100, Ralph Böhme via samba-technical wrote:
> Hi folks,
> 
> On Thu, Dec 21, 2017 at 03:06:32PM -0800, Jeremy Allison wrote:
> > On Thu, Dec 21, 2017 at 11:23:01PM +0100, Ralph Böhme wrote:
> > > 
> > > The one I have is "[PATCHES] Samba-VirusFilter (version 6)". Is that the latest one? If
> > > not, can you please bounce me their latest mail to s-t so I can follow up on the
> > > list? Thanks!
> > 
> > Here's the latest version I have.
> 
> Jeremy, attached is the rewritten version as discussed.
> 
> Trevor, Satoh, a few days ago Jeremy asked me to look into the latest version of
> the Virusfilter patchset (probably version 10 as posted on
> https://lists.samba.org/archive/samba-technical/2017-July/121573.html), asking
> for my opinion on the code.
> 
> I haven't been involved in the previous review rounds and somehow felt that the
> current implementation using #includes of some generic code into three VFS
> modules looked awkward.
> 
> I got bored the other day on a train from Frankenberg to Goslar, so I took the
> liberty to bend the code to my will. Attached is a rewrite that removes the
> #include magic by splitting the modules into one frontend VFS moulde
> vfs_virusfilter and three scanner backends. The backend is then chosen by a new
> option "virusfilter:scanner=clamav|fsav|sophos".
> 
> Trevor and Satoh, can you take a look and tell us if you agree with the rewrite?
> This is the general design change that Jeremy and I agreed upon to be necessary
> for the module to go upstream.
> 
> I tried to be careful when shuffling around the options and removing all the
> macro stuff, so hopefully everything still works, but given the size of the
> change this needs some testing I guess. The attached patchset compiles, but
> hasn't seen any testing...
> 
> The attach patchset keeps the FIXUP patches as seperate patches on top of the
> individual original patches. This is just to make it easier to see what I'm
> doing. If you're agree with the new design, just fold the fixups into the
> preceding patch.
> 
> If yes, then I guess there's little left that hinders final review by me and
> possibly Jeremy. There are still lots of README.Coding violations, I've already
> fixed many of them in the rewrite, I'd appreciate if you could fix the
> remaining. Most of the violations are function calls in conditionals:
> 
> if (somefunc() COMPARISON EXPECTED_RESULT) { ... }
> 
> Please rewrite as:
> 
> VARIABLE = somefunc();
> if (VARIABLE COMPARISON EXPECTED_RESULT) { ... }

Ralph, thanks SO MUCH for that refactoring. It addresses just
about all the concerns I had with the patchset, and makes the
logic a lot easier to understand, which was the main reason
I was sat on this thing for so long.

Trevor, Satoh, can you confirm that this reworked set still
works for what you need ? If so, then I will work with you
(and get final Ralph review) for the README.Coding violations
and we can (finally) get this upstream.

Cheers,

	Jeremy.