Request for review of Samba VirusFilter preparation for merging with Samba

Jeremy Allison jra at samba.org
Sat Sep 10 18:54:21 UTC 2016


On Sat, Sep 10, 2016 at 12:00:26PM -0600, Trever L. Adams wrote:
> >
> > Then -- no lines beyond 80 chars please.
> Is this hard and fast? There are a few things that are right around
> 80-83 characters that would be difficult to split up.

Yeah, it is really. Just break the lines up.

> Does this apply to function definitions in .h files?
> >
> > The virusfilter_io abstraction could see a lot of code re-use from our
> > base libraries. We have for example write_data_iov that you could
> > re-use. If that does not do what you need, can we share this code
> > somehow?
> I will look into that and work with the author/maintainer.
> >
> >
> > If I get the code right, you're not linking to proprietary libaries
> > but connect via sockets to an AV daemon. Is that right? We never
> > really approached AV because that usually means linking to proprietary
> > libraries. This problem is gone now?
> >
> > Volker
> >
> I only worked with the clamav part, but I quickly looked over the
> others, as far as I know there is no linking, only connecting via
> sockets. I believe there was a planned module to do external calls
> (fork/exec/std(in|out) redirection), but I don't believe that has been
> done, just planned.
> 
> Other than write_data_iov and other reuse issues, I believe I have done
> as you have asked.

Looking at vfs_virusfilter_vfs.c you are now mixing talloc with SAFE_FREE.
That will not end well :-).

Also there need to be NULL return checks on all uses of talloc.
They are missing in vfs_virusfilter_clamav.c for example.

Hope you don't think I'm being too picky. This is *really*
great work and I'm very happy you're doing it. I'd certainly
like to get this in a state where we can merge and support
it formally.

Cheers,

	Jeremy.



More information about the samba-technical mailing list