netdev - Re: [PATCH] [CAIF-RFC 4/8-v2] CAIF Protocol Stack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AD31EF1.2010102@babic.homelinux.org>
Date:	Mon, 12 Oct 2009 14:20:01 +0200
From:	Stefano Babic <stefano.babic@...ic.homelinux.org>
To:	sjur.brandeland@...ricsson.com
CC:	netdev@...r.kernel.org, kim.xx.lilliestierna@...ricsson.com,
	christian.bejram@...ricsson.com, daniel.martensson@...ricsson.com
Subject: Re: [PATCH] [CAIF-RFC 4/8-v2] CAIF Protocol Stack

sjur.brandeland@...ricsson.com wrote:
> From: Sjur Braendeland <sjur.brandeland@...ricsson.com>

Hi Sjur,

> 
> diff --git a/net/caif/generic/cfcnfg.c b/net/caif/generic/cfcnfg.c
> new file mode 100644
> index 0000000..3aad201

> + * NOTE: What happends destroy failure:

s/happends/happens/, there are other occurencies.

> +
> +bool cfcnfg_del_adapt_layer(struct _cfcnfg_t *cnfg, layer_t *adap_layer)
> +{
> +	uint8 channel_id = 0;
> +	struct cfcnfg_phyinfo *phyinfo = NULL;
> +	uint8 phyid = 0;
> +	CFLOG_TRACE(("cfcnfg: enter del_adaptation_layer\n"));
> +
> +	cfglu_assert(adap_layer != NULL);
> +	channel_id = adap_layer->id;
> +	cfglu_assert(channel_id != 0);

My two cents about using assert in the code, but I prefer to get some
info from system when something goes wrong as to call in some way panic
(the assert calls BUG_ON) and blocks forever.
This line is not different as checking the adapt_layer->dn pointer some
lines after and I think an error is better recognized in that case. So
IMHO should be better something like:

	if (channel_id == 0) {
		CFLOG_ERROR(("cfcnfg:adap_layer->dn is NULL\n"));
		return CFGLU_EINVAL;
	}


> diff --git a/net/caif/generic/cfpkt_plain.c b/net/caif/generic/cfpkt_plain.c
> +#define CHECK_MEM 1

Probably you plan to use always the magic number in your buffer
management. Then should be better to remove all the #if CHECK_MEM stuff.


> +#if CHECK_MEM
> +
> +#endif

This three lines seem not needed.... ;)

Stefano

-- 
stefano <stefano.babic@...ic.homelinux.org>
GPG Key: 0x55814DDE
Fingerprint 4E85 2A66 4CBA 497A 2A7B D3BF 5973 F216 5581 4DDE
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html