Request for review of Samba VirusFilter preparation for merging with Samba

Volker Lendecke vl at samba.org
Sat Sep 10 09:41:50 UTC 2016


On Fri, Sep 09, 2016 at 11:15:04AM -0600, Trever L. Adams wrote:
> I have done some previous work on this. As stated, I am not the original
> author or maintainer, but I am working with him to knock out his TODO
> list and get it ready for merging. I believe we may be in the last week
> or so of work on this.
> 
> Anyone who works with integrating modules who is willing to work with me
> (knowing I am only helping out), please review
> https://github.com/fumiyas/samba-virusfilter/issues/23 and the github
> tree
> https://github.com/treveradams/samba-virusfilter/tree/merge_prep_reorganize.
> 
> I would love to have input so that I can do my part in this as quickly,
> efficiently and correctly as possible.

Just took a brief look at vfs_virusfilter_vfs.c. A few comments:

Don't use SMB_STRDUP and SMB_MALLOC in new code. It's all talloc now
with a proper talloc hierarchy or for tmp variables with talloc_tos().

"if (q_prefix) TALLOC_FREE(q_prefix);" is double-checking. TALLOC_FREE
already checks for NULL.

There's a LOT of become_root() inside. To be honest, I feel a bit
uneasy with that. Can you explain why that is required?

Then -- no lines beyond 80 chars please.

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?


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



More information about the samba-technical mailing list