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@xxxxxxxxx> --- 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