From:   Armin Wolf <W_Armin@gmx.de>
To:     pali@kernel.org
Cc:     jdelvare@suse.com, linux@roeck-us.net, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: [PATCH v2 1/2] hwmon: (dell-smm) Simplify ioctl handler
Date:   Sat, 11 Dec 2021 16:54:21 +0100
Message-Id: <20211211155422.16830-2-W_Armin@gmx.de>
In-Reply-To: <20211211155422.16830-1-W_Armin@gmx.de>
References: <20211211155422.16830-1-W_Armin@gmx.de>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

The second switch-case has no real purpose:

- for I8K_BIOS_VERSION, val does not represent a return value,
  making the check for error values unnecessary.
- for I8K_MACHINE_ID, val remains zero, so the error check is
  unnecessary too.

Remove the switch-case and move the calls to copy_to_user()
into the first switch-case for I8K_BIOS_VERSION/_MACHINE_ID.
Omit buff[] since data->bios_machineid already contains the string
with the necessary zero padding through devm_kzalloc().

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
=2D--
 drivers/hwmon/dell-smm-hwmon.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon=
.c
index 5596c211f38d..186d40938036 100644
=2D-- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -454,7 +454,6 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_da=
ta *data, unsigned int cmd
 {
 	int val =3D 0;
 	int speed, err;
-	unsigned char buff[16];
 	int __user *argp =3D (int __user *)arg;

 	if (!argp)
@@ -468,15 +467,19 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_=
data *data, unsigned int cmd

 		val =3D (data->bios_version[0] << 16) |
 				(data->bios_version[1] << 8) | data->bios_version[2];
-		break;

+		if (copy_to_user(argp, &val, sizeof(val)))
+			return -EFAULT;
+
+		return 0;
 	case I8K_MACHINE_ID:
 		if (restricted && !capable(CAP_SYS_ADMIN))
 			return -EPERM;

-		strscpy_pad(buff, data->bios_machineid, sizeof(buff));
-		break;
+		if (copy_to_user(argp, data->bios_machineid, sizeof(data->bios_machinei=
d)))
+			return -EFAULT;

+		return 0;
 	case I8K_FN_STATUS:
 		val =3D i8k_get_fn_status();
 		break;
@@ -527,23 +530,8 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_d=
ata *data, unsigned int cmd
 	if (val < 0)
 		return val;

-	switch (cmd) {
-	case I8K_BIOS_VERSION:
-		if (copy_to_user(argp, &val, 4))
-			return -EFAULT;
-
-		break;
-	case I8K_MACHINE_ID:
-		if (copy_to_user(argp, buff, 16))
-			return -EFAULT;
-
-		break;
-	default:
-		if (copy_to_user(argp, &val, sizeof(int)))
-			return -EFAULT;
-
-		break;
-	}
+	if (copy_to_user(argp, &val, sizeof(int)))
+		return -EFAULT;

 	return 0;
 }
=2D-
2.30.2