[PATCH 1/1] HID: hiddev: remove a duplicated check

[PATCH 1/1] HID: hiddev: remove a duplicated check

[Zhen Lei]

After commit 9c09b214f30e ("HID: hiddev: avoid opening a disconnected
device"), the original check "list->hiddev->exist" become unnecessary.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/hid/usbhid/hiddev.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index e421cdf2d1a4ec5..e04cf95fca094a8 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -292,16 +292,16 @@ static int hiddev_open(struct inode *inode, struct file *file)
 		res = -ENODEV;
 		goto bail_unlock;
 	}
-	if (!list->hiddev->open++)
-		if (list->hiddev->exist) {
-			struct hid_device *hid = hiddev->hid;
-			res = hid_hw_power(hid, PM_HINT_FULLON);
-			if (res < 0)
-				goto bail_unlock;
-			res = hid_hw_open(hid);
-			if (res < 0)
-				goto bail_normal_power;
-		}
+	if (!list->hiddev->open++) {
+		struct hid_device *hid = hiddev->hid;
+
+		res = hid_hw_power(hid, PM_HINT_FULLON);
+		if (res < 0)
+			goto bail_unlock;
+		res = hid_hw_open(hid);
+		if (res < 0)
+			goto bail_normal_power;
+	}
 	mutex_unlock(&hiddev->existancelock);
 	return 0;
 bail_normal_power:
-- 
1.8.3

Re: [PATCH 1/1] HID: hiddev: remove a duplicated check

[Leizhen (ThunderTown)]

Does no one pay attention to this problem? Although there is no
functional problem, but it seems confusing.

        if (!list->hiddev->exist) {                 <---------
                res = -ENODEV;                                |
                goto bail_unlock;                             |
        }                                                     |
        if (!list->hiddev->open++)                            |
                if (list->hiddev->exist) {          <--------- //It's always true.

On 2019/12/24 11:51, Zhen Lei wrote:
> After commit 9c09b214f30e ("HID: hiddev: avoid opening a disconnected
> device"), the original check "list->hiddev->exist" become unnecessary.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/hid/usbhid/hiddev.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index e421cdf2d1a4ec5..e04cf95fca094a8 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -292,16 +292,16 @@ static int hiddev_open(struct inode *inode, struct file *file)
>  		res = -ENODEV;
>  		goto bail_unlock;
>  	}
> -	if (!list->hiddev->open++)
> -		if (list->hiddev->exist) {
> -			struct hid_device *hid = hiddev->hid;
> -			res = hid_hw_power(hid, PM_HINT_FULLON);
> -			if (res < 0)
> -				goto bail_unlock;
> -			res = hid_hw_open(hid);
> -			if (res < 0)
> -				goto bail_normal_power;
> -		}
> +	if (!list->hiddev->open++) {
> +		struct hid_device *hid = hiddev->hid;
> +
> +		res = hid_hw_power(hid, PM_HINT_FULLON);
> +		if (res < 0)
> +			goto bail_unlock;
> +		res = hid_hw_open(hid);
> +		if (res < 0)
> +			goto bail_normal_power;
> +	}
>  	mutex_unlock(&hiddev->existancelock);
>  	return 0;
>  bail_normal_power:
> 

Re: [PATCH 1/1] HID: hiddev: remove a duplicated check

[Jiri Kosina]

On Tue, 14 Jan 2020, Leizhen (ThunderTown) wrote:

> Does no one pay attention to this problem? Although there is no
> functional problem, but it seems confusing.
> 
>         if (!list->hiddev->exist) {                 <---------
>                 res = -ENODEV;                                |
>                 goto bail_unlock;                             |
>         }                                                     |
>         if (!list->hiddev->open++)                            |
>                 if (list->hiddev->exist) {          <--------- //It's always true.

This code no longer exists after refactoring that happened in commit 
18a1b06e5b91d47.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/1] HID: hiddev: remove a duplicated check

[Leizhen (ThunderTown)]

On 2020/1/14 22:19, Jiri Kosina wrote:
> On Tue, 14 Jan 2020, Leizhen (ThunderTown) wrote:
> 
>> Does no one pay attention to this problem? Although there is no
>> functional problem, but it seems confusing.
>>
>>         if (!list->hiddev->exist) {                 <---------
>>                 res = -ENODEV;                                |
>>                 goto bail_unlock;                             |
>>         }                                                     |
>>         if (!list->hiddev->open++)                            |
>>                 if (list->hiddev->exist) {          <--------- //It's always true.
> 
> This code no longer exists after refactoring that happened in commit 
> 18a1b06e5b91d47.
OK

>