linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	brgl@bgdev.pl, linus.walleij@linaro.org
Cc: Kent Gibson <warthog618@gmail.com>
Subject: [PATCH 3/3] gpiolib: cdev: Cleanup kfifo_out() error handling
Date: Mon, 27 May 2024 19:54:19 +0800	[thread overview]
Message-ID: <20240527115419.92606-4-warthog618@gmail.com> (raw)
In-Reply-To: <20240527115419.92606-1-warthog618@gmail.com>

The handling of kfifo_out() errors in read functions obscures any error.
The error condition should never occur but, while a ret is set to -EIO, it
is subsequently ignored and the read functions instead return the number
of bytes copied to that point, potentially masking the fact that any error
occurred.

Return -EIO in the case of a kfifo_out() error to make it clear something
very odd is going on here.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 47 +++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index c7218c9f2c5e..6a986d7f1f2f 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1642,16 +1642,13 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
 					return ret;
 			}
 
-			ret = kfifo_out(&lr->events, &le, 1);
-		}
-		if (ret != 1) {
-			/*
-			 * This should never happen - we were holding the
-			 * lock from the moment we learned the fifo is no
-			 * longer empty until now.
-			 */
-			ret = -EIO;
-			break;
+			if (kfifo_out(&lr->events, &le, 1) != 1)
+				/*
+				 * This should never happen - we hold the
+				 * lock from the moment we learned the fifo
+				 * is no longer empty until now.
+				 */
+				return -EIO;
 		}
 
 		if (copy_to_user(buf + bytes_read, &le, sizeof(le)))
@@ -1995,16 +1992,13 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 					return ret;
 			}
 
-			ret = kfifo_out(&le->events, &ge, 1);
-		}
-		if (ret != 1) {
-			/*
-			 * This should never happen - we were holding the lock
-			 * from the moment we learned the fifo is no longer
-			 * empty until now.
-			 */
-			ret = -EIO;
-			break;
+			if (kfifo_out(&le->events, &ge, 1) != 1)
+				/*
+				 * This should never happen - we hold the
+				 * lock from the moment we learned the fifo
+				 * is no longer empty until now.
+				 */
+				return -EIO;
 		}
 
 		if (copy_to_user(buf + bytes_read, &ge, ge_size))
@@ -2707,12 +2701,13 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 			if (count < event_size)
 				return -EINVAL;
 #endif
-			ret = kfifo_out(&cdev->events, &event, 1);
-		}
-		if (ret != 1) {
-			ret = -EIO;
-			break;
-			/* We should never get here. See lineevent_read(). */
+			if (kfifo_out(&cdev->events, &event, 1) != 1)
+				/*
+				 * This should never happen - we hold the
+				 * lock from the moment we learned the fifo
+				 * is no longer empty until now.
+				 */
+				return -EIO;
 		}
 
 #ifdef CONFIG_GPIO_CDEV_V1
-- 
2.39.2


  parent reply	other threads:[~2024-05-27 11:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-27 11:54 [PATCH 0/3] gpiolib: cdev: tidy up kfifo handling Kent Gibson
2024-05-27 11:54 ` [PATCH 1/3] gpiolib: cdev: Add INIT_KFIFO() for linereq events Kent Gibson
2024-05-27 11:54 ` [PATCH 2/3] gpiolib: cdev: Refactor allocation of linereq events kfifo Kent Gibson
2024-05-27 11:54 ` Kent Gibson [this message]
2024-05-29 11:24   ` [PATCH 3/3] gpiolib: cdev: Cleanup kfifo_out() error handling Bartosz Golaszewski
2024-05-29 11:44     ` Kent Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240527115419.92606-4-warthog618@gmail.com \
    --to=warthog618@gmail.com \
    --cc=brgl@bgdev.pl \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).