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

[Joe Perches]

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@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel