[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240905041732.2034348-21-dmitry.torokhov@gmail.com>
Date: Wed, 4 Sep 2024 21:17:25 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: linux-input@...r.kernel.org
Cc: Pali Rohár <pali@...nel.org>,
Helge Deller <deller@....de>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>,
Samuel Holland <samuel@...lland.org>,
Lyude Paul <thatslyude@...il.com>,
Michal Simek <michal.simek@....com>,
Hans de Goede <hdegoede@...hat.com>,
linux-kernel@...r.kernel.org,
linux-hyperv@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev
Subject: [PATCH 20/24] Input: serio_raw - use guard notation for locks and other resources
Use guard notation when acquiring mutexes and spinlocks, and when
pausing and resuming serio port. Such guard notation makes the code
more compact and error handling more robust by ensuring that locks
are released in all code paths when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
---
drivers/input/serio/serio_raw.c | 121 +++++++++++++-------------------
1 file changed, 49 insertions(+), 72 deletions(-)
diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 0186d1b38f49..aef8301313b2 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -75,41 +75,31 @@ static int serio_raw_open(struct inode *inode, struct file *file)
{
struct serio_raw *serio_raw;
struct serio_raw_client *client;
- int retval;
- retval = mutex_lock_interruptible(&serio_raw_mutex);
- if (retval)
- return retval;
+ scoped_guard(mutex_intr, &serio_raw_mutex) {
+ serio_raw = serio_raw_locate(iminor(inode));
+ if (!serio_raw)
+ return -ENODEV;
- serio_raw = serio_raw_locate(iminor(inode));
- if (!serio_raw) {
- retval = -ENODEV;
- goto out;
- }
+ if (serio_raw->dead)
+ return -ENODEV;
- if (serio_raw->dead) {
- retval = -ENODEV;
- goto out;
- }
+ client = kzalloc(sizeof(*client), GFP_KERNEL);
+ if (!client)
+ return -ENOMEM;
- client = kzalloc(sizeof(*client), GFP_KERNEL);
- if (!client) {
- retval = -ENOMEM;
- goto out;
- }
+ client->serio_raw = serio_raw;
+ file->private_data = client;
- client->serio_raw = serio_raw;
- file->private_data = client;
+ kref_get(&serio_raw->kref);
- kref_get(&serio_raw->kref);
+ scoped_guard(serio_pause_rx, serio_raw->serio)
+ list_add_tail(&client->node, &serio_raw->client_list);
- serio_pause_rx(serio_raw->serio);
- list_add_tail(&client->node, &serio_raw->client_list);
- serio_continue_rx(serio_raw->serio);
+ return 0;
+ }
-out:
- mutex_unlock(&serio_raw_mutex);
- return retval;
+ return -EINTR;
}
static void serio_raw_free(struct kref *kref)
@@ -126,9 +116,8 @@ static int serio_raw_release(struct inode *inode, struct file *file)
struct serio_raw_client *client = file->private_data;
struct serio_raw *serio_raw = client->serio_raw;
- serio_pause_rx(serio_raw->serio);
- list_del(&client->node);
- serio_continue_rx(serio_raw->serio);
+ scoped_guard(serio_pause_rx, serio_raw->serio)
+ list_del(&client->node);
kfree(client);
@@ -139,19 +128,15 @@ static int serio_raw_release(struct inode *inode, struct file *file)
static bool serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
{
- bool empty;
+ guard(serio_pause_rx)(serio_raw->serio);
- serio_pause_rx(serio_raw->serio);
-
- empty = serio_raw->head == serio_raw->tail;
- if (!empty) {
- *c = serio_raw->queue[serio_raw->tail];
- serio_raw->tail = (serio_raw->tail + 1) % SERIO_RAW_QUEUE_LEN;
- }
+ if (serio_raw->head == serio_raw->tail)
+ return false; /* queue is empty */
- serio_continue_rx(serio_raw->serio);
+ *c = serio_raw->queue[serio_raw->tail];
+ serio_raw->tail = (serio_raw->tail + 1) % SERIO_RAW_QUEUE_LEN;
- return !empty;
+ return true;
}
static ssize_t serio_raw_read(struct file *file, char __user *buffer,
@@ -200,40 +185,32 @@ static ssize_t serio_raw_write(struct file *file, const char __user *buffer,
{
struct serio_raw_client *client = file->private_data;
struct serio_raw *serio_raw = client->serio_raw;
- int retval = 0;
+ int written;
unsigned char c;
- retval = mutex_lock_interruptible(&serio_raw_mutex);
- if (retval)
- return retval;
+ scoped_guard(mutex_intr, &serio_raw_mutex) {
+ if (serio_raw->dead)
+ return -ENODEV;
- if (serio_raw->dead) {
- retval = -ENODEV;
- goto out;
- }
+ if (count > 32)
+ count = 32;
- if (count > 32)
- count = 32;
+ while (count--) {
+ if (get_user(c, buffer++))
+ return -EFAULT;
- while (count--) {
- if (get_user(c, buffer++)) {
- retval = -EFAULT;
- goto out;
- }
+ if (serio_write(serio_raw->serio, c)) {
+ /* Either signal error or partial write */
+ return written ?: -EIO;
+ }
- if (serio_write(serio_raw->serio, c)) {
- /* Either signal error or partial write */
- if (retval == 0)
- retval = -EIO;
- goto out;
+ written++;
}
- retval++;
+ return written;
}
-out:
- mutex_unlock(&serio_raw_mutex);
- return retval;
+ return -EINTR;
}
static __poll_t serio_raw_poll(struct file *file, poll_table *wait)
@@ -379,10 +356,10 @@ static void serio_raw_hangup(struct serio_raw *serio_raw)
{
struct serio_raw_client *client;
- serio_pause_rx(serio_raw->serio);
- list_for_each_entry(client, &serio_raw->client_list, node)
- kill_fasync(&client->fasync, SIGIO, POLL_HUP);
- serio_continue_rx(serio_raw->serio);
+ scoped_guard(serio_pause_rx, serio_raw->serio) {
+ list_for_each_entry(client, &serio_raw->client_list, node)
+ kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+ }
wake_up_interruptible(&serio_raw->wait);
}
@@ -394,10 +371,10 @@ static void serio_raw_disconnect(struct serio *serio)
misc_deregister(&serio_raw->dev);
- mutex_lock(&serio_raw_mutex);
- serio_raw->dead = true;
- list_del_init(&serio_raw->node);
- mutex_unlock(&serio_raw_mutex);
+ scoped_guard(mutex, &serio_raw_mutex) {
+ serio_raw->dead = true;
+ list_del_init(&serio_raw->node);
+ }
serio_raw_hangup(serio_raw);
--
2.46.0.469.g59c65b2a67-goog
Powered by blists - more mailing lists