PATCH: Samba-VirusFilter

[Trever L. Adams]

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>