PATCH: Samba-VirusFilter
Trever L. Adams
trever at middleearth.sapphiresunday.org
Mon Oct 17 19:09:11 UTC 2016
On 10/17/2016 11:35 AM, Alexander Bokovoy wrote:
> Hi Trever,
>
> I think there are few more places were clean up is needed. See below.
>
> On ma, 17 loka 2016, Trever L. Adams wrote:
>> On 10/16/2016 12:54 PM, SATOH Fumiyasu wrote:
>>> On Fri, 14 Oct 2016 22:08:03 +0900,
>>> Trever L. Adams wrote:
>>>> [1 PATCH: Samba-VirusFilter <multipart/mixed (7bit)>]
>>>> [1.1 <text/plain; utf-8 (quoted-printable)>]
>>>> This patch relies on the one in the message with subject "Proposed
>>>> Patch: write_data_iov_timeout, etc." Beyond that, it should be atomic.
>>> Signed-off-by: SATOH Fumiyasu <fumiyas at osstech.co.jp>
>> I missed a few coding style related cleanups. Also, the become_root and
>> unbecome_root in the quarantine action has been cleaned up some. I now
>> only drop it in one place. Before I dropped it in two and one was
>> actually a few lines early.
>>
>> I didn't merge the become_root conditionally or unconditionally with the
>> block below as it is my understanding that it is preferred that
>> become_root is not conditional and that it should cover the minimal
>> amount of code.
>>
>> I can change this if it is preferred.
>>
>> Thank you.
>>
>> Trever
>> diff --git a/buildtools/wafsamba/stale_files.py b/buildtools/wafsamba/stale_files.py
>> index 2dd08e1..0afbfe4 100644
>> --- a/buildtools/wafsamba/stale_files.py
>> +++ b/buildtools/wafsamba/stale_files.py
>> @@ -88,7 +88,7 @@ def replace_refill_task_list(self):
>> link = os.readlink(p)
>> if link[0:bin_base_len] == bin_base:
>> p = link
>> - if f in ['config.h']:
>> + if f in ['config.h', 'vfs_virusfilter_config.h']:
>> continue
>> (froot, fext) = os.path.splitext(f)
>> if fext not in [ '.c', '.h', '.so', '.o' ]:
>> diff --git a/docs-xml/manpages/vfs_virusfilter_clamav.8.xml b/docs-xml/manpages/vfs_virusfilter_clamav.8.xml
>> new file mode 100644
>> index 0000000..2483368
>> --- /dev/null
>> +++ b/docs-xml/manpages/vfs_virusfilter_clamav.8.xml
>> @@ -0,0 +1,295 @@
>> +<?xml version="1.0" encoding="iso-8859-1"?>
>> +<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
>> +<refentry id="vfs_virusfilter_clamav.8">
>> +
>> +<refmeta>
>> + <refentrytitle>vfs_virusfilter_clamav</refentrytitle>
>> + <manvolnum>8</manvolnum>
>> + <refmiscinfo class="source">Samba</refmiscinfo>
>> + <refmiscinfo class="manual">System Administration tools</refmiscinfo>
>> + <refmiscinfo class="version">4.4</refmiscinfo>
>> +</refmeta>
> This is going to git master, so update the version to 4.6, please.
Thank you. Done.
> +#if 0 /* FIXME */
> + if (virusfilter_io_writefl_readl(io_h,
> + "CONFIGURE\tTIMEOUT\t%d", virusfilter_h->timeout / 1000)
> + != VIRUSFILTER_RESULT_OK) {
> + DEBUG(0,("fsavd: CONFIGURE TIMEOUT: I/O error: %s\n",
> + strerror(errno)));
> + goto virusfilter_fsav_init_failed;
> + }
> + if (!strn_eq(io_h->r_buffer, "OK\t", 3)) {
> + DEBUG(0,("fsavd: CONFIGURE TIMEOUT: Not accepted: %s\n",
> + io_h->r_buffer));
> + goto virusfilter_fsav_init_failed;
> + }
> +#endif
> Could you please either add a more detailed comment what's broken here
> or remove this '#if 0' at all?
I have removed them all.
>
>> + } else if (str_eq(reply_token, "SUSPECTED") ||
>> + str_eq(reply_token, "ARCHIVE_SUSPECTED") ||
>> + str_eq(reply_token, "MIME_SUSPECTED")) {
>> +#if 0
>> + // FIXME: Block if "block suspected file" option is
>> + // true
>> + result = VIRUSFILTER_RESULT_SUSPECTED;
>> + ...
>> +#else
>> + /* Ignore */
>> +#endif
> Could you please use single style for comments?
> (There are more places like this)
I believe all comments now follow the coding style completely. No C++
style comments are left.
>
>> + }
>> + *dst_c++ = *src;
>> + dst_size--;
>> + }
>> + }
>> +
>> + *dst_c = '\0';
>> +
>> + return (dst_c - dst);
>> +}
>> +
>> +#if SAMBA_VERSION_NUMBER >= 30600
>> +/*********************************************************
>> + For rename across filesystems initial Patch from Warren Birnbaum
>> + <warrenb at hpcvscdp.cv.hp.com>
>> +**********************************************************/
> It is good to have compatibility code but if we are going to merge this
> to git master, can we remove old code or wrapping against the old code?
> (There are more places like this)
All compatibility code has been removed.
Thank you for the review.
Trever
-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba-virusfilter.patch
Type: text/x-patch
Size: 151598 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161017/f880dc0d/samba-virusfilter.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 872 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161017/f880dc0d/signature.sig>
More information about the samba-technical
mailing list