Re: [2.6 patch] drivers/ieee1394/csr1212.c: remove dead code

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

Adrian Bunk wrote:
> The Coverity checker spotted that the same check was already done above.
> 
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- linux-2.6.15-rc1-mm2-full/drivers/ieee1394/csr1212.c.old	2005-11-20 22:50:14.000000000 +0100
> +++ linux-2.6.15-rc1-mm2-full/drivers/ieee1394/csr1212.c	2005-11-20 22:50:36.000000000 +0100
> @@ -1616,12 +1616,8 @@
>  	 * and make cache regions for them */
>  	for (dentry = csr->root_kv->value.directory.dentries_head;
>  	     dentry; dentry = dentry->next) {
> -		if (dentry->kv->key.id == CSR1212_KV_ID_EXTENDED_ROM) {
> +		if (dentry->kv->key.id == CSR1212_KV_ID_EXTENDED_ROM)
>  			csr1212_get_keyval(csr, dentry->kv);
> -
> -			if (ret != CSR1212_SUCCESS)
> -				return ret;
> -		}
>  	}
>  
>  	return CSR1212_SUCCESS;

Yes, this is dead code. But when I looked through csr1212_parse_csr() 
which you are patching here, I wondered why the return code of 
csr1212_get_keyval() is never checked there. csr1212_get_keyval() 
performs memory allocations and bus reads. Shouldn't both calls of 
csr1212_get_keyval() be enclosed in something like this?

	if(!csr1212_get_keyval(...))
		return ~ CSR1212_SUCCESS;

Or for better yet, we should use _csr1212_read_keyval() instead so that 
we get more sensible error codes.
-- 
Stefan Richter
-=====-=-=-= =-== =-=-=
http://arcgraph.de/sr/