[PATCH] Samba VirusFilter (version 11)

jim jim.brown at rsmas.miami.edu
Thu Dec 28 15:41:24 UTC 2017


Trever
Status of functions returning virusfilter_result should be checked 
against correct constants (not true or false).
A few more nits.
Jim

Tests for type virusfilter_result should be against correct constants - 
not against true.
Or, it should be assigned to a bool variable like virusfilter_io_readl.

result != true should be result != VIRUSFILTER_RESULT_OK (many places)

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	become_root();
> +	result = virusfilter_io_connect_path(io_h, config->socket_path);
> +	unbecome_root();
> +
> +	if (result != true) {

Should be 'if (config == NULL) {'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	config = talloc_zero(handle, struct virusfilter_config);
> +	if (!config) {
> +		DBG_ERR("talloc_zero failed\n");

Missing space after 'if'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	if(tmp != config->quarantine_dir) {
> +		TALLOC_FREE(tmp);

Variables should be tested against 0 - 'if (... > 0) {'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	test_prefix = strlen(config->rename_prefix);
> +	test_suffix = strlen(config->rename_suffix);
> +	if (test_prefix) {
> +		rename_trap_count++;
> +	}
> +	if (test_suffix) {
> +		rename_trap_count++;
> +	}

Multiple places for text_prefix and test_suffix in this block should be 
'... > 0'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	if (test_prefix || test_suffix) {
> +		ok1 = parent_dirname(mem_ctx, fname, &dir_name, &base_name);

Should be 'if (iov_p->iov_base == NULL) {'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +		iov_p->iov_base = va_arg(ap, void *);
> +		if (!iov_p->iov_base) {
> +			break;

result is bool
Use 'if (!result) {'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +		if (result != true) {
> +			return result;
> +		}

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	result = virusfilter_io_readl(talloc_tos(), io_h, read_line);
> +	if (result != true) {
> +		DBG_ERR("virusfilter_io_readl not OK: %d\n", result);
> +		return false;
> +	}

Multiple places in two virus scanners should be 'reply_token != NULL' in 
if tests

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +			reply_token = strtok_r(NULL, " ", &reply_saveptr);
> +			if (reply_token) {

Should be 'if (reply_token == NULL) {'

On 12/28/2017 2:27 AM, Trever L. Adams via samba-technical wrote:
> +	reply_token = strrchr(reply, ' ');
> +
> +	if (!reply_token) {
> +		DBG_ERR("clamd: zSCAN: Invalid reply: %s\n",
> +			reply);




More information about the samba-technical mailing list