Re: [PATCH 8/9] staging: brcm80211: fix checkpatch error 'assignment in if condition'

[Joe Perches <joe@xxxxxxxxxxx>]

On Thu, 2010-09-30 at 10:43 -0400, Jason Cooper wrote:
>  drivers/staging/brcm80211/brcmfmac/wl_iw.c |  286 +++++++++++++++++-----------
> @@ -461,9 +470,14 @@ wl_iw_get_mode(struct net_device *dev,
>  
>  	WL_TRACE(("%s: SIOCGIWMODE\n", dev->name));
>  
> -	if ((error = dev_wlc_ioctl(dev, WLC_GET_INFRA, &infra, sizeof(infra)))
> -	    || (error = dev_wlc_ioctl(dev, WLC_GET_AP, &ap, sizeof(ap))))
> +	error = dev_wlc_ioctl(dev, WLC_GET_INFRA, &infra, sizeof(infra));
> +	if (error) {
>  		return error;
> +	} else {
> +		error = dev_wlc_ioctl(dev, WLC_GET_AP, &ap, sizeof(ap));
> +		if (error)
> +			return error;
> +	}

Hi Jason.

The added else is ugly.

There's nothing wrong with an if that has multiple "(error = func) ||"
uses.  It can save lines and is quite readable.

This change should either just be a style change moving the logical
test to the end of line like:

	if ((error = dev_wlc_ioctl(dev, WLC_GET_INFRA, &infra, sizeof(infra))) ||
	    (error = dev_wlc_ioctl(dev, WLC_GET_AP, &ap, sizeof(ap))))
		return error;

or it should be multiple lines like:

	error = dev_wlc_ioctl(dev, WLC_GET_INFRA, &infra, sizeof(infra));
	if (error)
		return error;

	error = dev_wlc_ioctl(dev, WLC_GET_AP, &ap, sizeof(ap));
	if (error)
		return error;

or not be changed at all.

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel