Latest AV code

[Jeremy Allison]

On Fri, Dec 22, 2017 at 02:27:23PM -0700, Trever L. Adams wrote:
> On 12/22/2017 02:21 PM, Jeremy Allison wrote:
> > 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.
> >
> Sure. I am not sure how to do a git clone of something like:
> 
> https://git.samba.org/?p=slow/samba.git;a=shortlog;h=refs/heads/av
> 
> 
> How would I do that so I can use it as a starting point?

You don't need to get Ralph's branch, he's provided everything
in the published patchset.

Easiest thing to do Trevor is to just use a copy of master,
then apply Ralph's patchset on top. You can then work on
that to squash the FIXUP's, then work on the README.Coding
changes. Then use:

git format-patch --stdout <starting-refspec>.. >/tmp/virus-patch

to get the modified patchset and re-post to samba-technical.

Cheers,

	Jeremy.