linux-kernel - Re: [PATCH 1/1] drivers: usb: core: devio.c: Coding style fixes

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <52598494.5020108@cogentembedded.com>
Date:	Sat, 12 Oct 2013 19:19:16 +0200
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Matthias Beyer <mail@...ermatthias.de>
CC:	gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] drivers: usb: core: devio.c: Coding style fixes

Hello.

On 12-10-2013 13:01, Matthias Beyer wrote:

> Whitespace fixes, including:

> - Whitespace after if-keyword
> - Spaces to Tabs

> Other coding style fixes:

> - Added braces for if-else blocks where missing

> Signed-off-by: Matthias Beyer <mail@...ermatthias.de>
> ---
>   drivers/usb/core/devio.c | 40 +++++++++++++++++++++-------------------
>   1 file changed, 21 insertions(+), 19 deletions(-)

> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 71dc5d7..5f8a40d 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
[...]
> @@ -1636,32 +1636,32 @@ static int proc_reapurbnonblock(struct dev_state *ps, void __user *arg)
>   static int proc_control_compat(struct dev_state *ps,
>   				struct usbdevfs_ctrltransfer32 __user *p32)
>   {
> -        struct usbdevfs_ctrltransfer __user *p;
> -        __u32 udata;
> -        p = compat_alloc_user_space(sizeof(*p));
> -        if (copy_in_user(p, p32, (sizeof(*p32) - sizeof(compat_caddr_t))) ||
> -            get_user(udata, &p32->data) ||
> +	struct usbdevfs_ctrltransfer __user *p;
> +	__u32 udata;

    Could you also add an empty line here, for consistency with the below code?

> +	p = compat_alloc_user_space(sizeof(*p));
> +	if (copy_in_user(p, p32, (sizeof(*p32) - sizeof(compat_caddr_t))) ||
> +	    get_user(udata, &p32->data) ||
>   	    put_user(compat_ptr(udata), &p->data))
>   		return -EFAULT;
> -        return proc_control(ps, p);
> +	return proc_control(ps, p);
>   }
>
>   static int proc_bulk_compat(struct dev_state *ps,
>   			struct usbdevfs_bulktransfer32 __user *p32)
>   {
> -        struct usbdevfs_bulktransfer __user *p;
> -        compat_uint_t n;
> -        compat_caddr_t addr;
> +	struct usbdevfs_bulktransfer __user *p;
> +	compat_uint_t n;
> +	compat_caddr_t addr;
>
> -        p = compat_alloc_user_space(sizeof(*p));
> +	p = compat_alloc_user_space(sizeof(*p));
>
> -        if (get_user(n, &p32->ep) || put_user(n, &p->ep) ||
> -            get_user(n, &p32->len) || put_user(n, &p->len) ||
> -            get_user(n, &p32->timeout) || put_user(n, &p->timeout) ||
> -            get_user(addr, &p32->data) || put_user(compat_ptr(addr), &p->data))
> -                return -EFAULT;
> +	if (get_user(n, &p32->ep) || put_user(n, &p->ep) ||
> +	    get_user(n, &p32->len) || put_user(n, &p->len) ||
> +	    get_user(n, &p32->timeout) || put_user(n, &p->timeout) ||
> +	    get_user(addr, &p32->data) || put_user(compat_ptr(addr), &p->data))
> +		return -EFAULT;
>
> -        return proc_bulk(ps, p);
> +	return proc_bulk(ps, p);
>   }
>   static int proc_disconnectsignal_compat(struct dev_state *ps, void __user *arg)
>   {
> @@ -1838,10 +1838,12 @@ static int proc_ioctl(struct dev_state *ps, struct usbdevfs_ioctl *ctl)
>   		return -ENODEV;
>   	}
>
> -	if (ps->dev->state != USB_STATE_CONFIGURED)
> +	if (ps->dev->state != USB_STATE_CONFIGURED) {
>   		retval = -EHOSTUNREACH;
> -	else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> +	}
> +	else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno))) {

    No, } and *else* should be on the same line. But wait, both arms of *if* 
are single statements, so why you added {} in the first place? :-/

>   		retval = -EINVAL;
> +	}

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/