[PATCH v2] media: vb2: use ssize_t for vb2_read/vb2_write

Zile Xiong posted 1 patch 2 weeks ago
There is a newer version of this series
drivers/media/common/videobuf2/videobuf2-core.c | 6 +++---
include/media/videobuf2-core.h                  | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
[PATCH v2] media: vb2: use ssize_t for vb2_read/vb2_write
Posted by Zile Xiong 2 weeks ago
vb2_read() and vb2_write() return size_t, but propagate
negative errno values from __vb2_perform_fileio() via
implicit signed/unsigned conversions in callers
(e.g. vb2_fop_read()), which is not obvious.

vb2_fop_read() and vb2_fop_write() already return ssize_t, so
using size_t for vb2_read(), vb2_write(), and
__vb2_perform_fileio() is inconsistent.

Switch these helpers to ssize_t so they can return either a byte
count or a negative error code.

Fixes: b25748fe6126 ("[media] v4l: videobuf2: add read() and write() emulator")
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Zile Xiong <xiongzile99@gmail.com>

v2:
- add Fixes tag (suggested by Marek Szyprowski)
---
 drivers/media/common/videobuf2/videobuf2-core.c | 6 +++---
 include/media/videobuf2-core.h                  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index adf668b213c2..8a2b8156e9da 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2990,7 +2990,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q)
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  * @read:	access mode selector (1 means read, 0 means write)
  */
-static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_t count,
+static ssize_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_t count,
 		loff_t *ppos, int nonblock, int read)
 {
 	struct vb2_fileio_data *fileio;
@@ -3154,14 +3154,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	return ret;
 }
 
-size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
+ssize_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
 		loff_t *ppos, int nonblocking)
 {
 	return __vb2_perform_fileio(q, data, count, ppos, nonblocking, 1);
 }
 EXPORT_SYMBOL_GPL(vb2_read);
 
-size_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
+ssize_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
 		loff_t *ppos, int nonblocking)
 {
 	return __vb2_perform_fileio(q, (char __user *) data, count,
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 4424d481d7f7..bf20bcb1d366 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -1093,7 +1093,7 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
  * @ppos:	file handle position tracking pointer
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  */
-size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
+ssize_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
 		loff_t *ppos, int nonblock);
 /**
  * vb2_write() - implements write() syscall logic.
@@ -1103,7 +1103,7 @@ size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
  * @ppos:	file handle position tracking pointer
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  */
-size_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
+ssize_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
 		loff_t *ppos, int nonblock);
 
 /**
-- 
2.39.5
[PATCH v3] media: vb2: use ssize_t for vb2_read/vb2_write
Posted by Zile Xiong 2 weeks ago
vb2_read() and vb2_write() return size_t, but propagate
negative errno values from __vb2_perform_fileio() via
implicit signed/unsigned conversions in callers
(e.g. vb2_fop_read()), which is not obvious.

vb2_fop_read() and vb2_fop_write() already return ssize_t, so
using size_t for vb2_read(), vb2_write(), and
__vb2_perform_fileio() is inconsistent.

Switch these helpers to ssize_t so they can return either a byte
count or a negative error code.

Fixes: b25748fe6126 ("[media] v4l: videobuf2: add read() and write() emulator")
Cc: stable@vger.kernel.org
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Zile Xiong <xiongzile99@gmail.com>
---
v3:
- add Cc: stable@vger.kernel.org
- fix function argument alignment

v2:
- add Fixes tag
- clarify type consistency with vb2_fop_read/write
---
 drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++------
 include/media/videobuf2-core.h                  |  8 ++++----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index adf668b213c2..d5c3d4d939aa 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2990,8 +2990,9 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q)
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  * @read:	access mode selector (1 means read, 0 means write)
  */
-static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_t count,
-		loff_t *ppos, int nonblock, int read)
+static ssize_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data,
+				    size_t count, loff_t *ppos,
+				    int nonblock, int read)
 {
 	struct vb2_fileio_data *fileio;
 	struct vb2_fileio_buf *buf;
@@ -3154,15 +3155,15 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 	return ret;
 }
 
-size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
-		loff_t *ppos, int nonblocking)
+ssize_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
+		 loff_t *ppos, int nonblocking)
 {
 	return __vb2_perform_fileio(q, data, count, ppos, nonblocking, 1);
 }
 EXPORT_SYMBOL_GPL(vb2_read);
 
-size_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
-		loff_t *ppos, int nonblocking)
+ssize_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
+		  loff_t *ppos, int nonblocking)
 {
 	return __vb2_perform_fileio(q, (char __user *) data, count,
 							ppos, nonblocking, 0);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 4424d481d7f7..4b4f4c15c53a 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -1093,8 +1093,8 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
  * @ppos:	file handle position tracking pointer
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  */
-size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
-		loff_t *ppos, int nonblock);
+ssize_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
+		 loff_t *ppos, int nonblock);
 /**
  * vb2_write() - implements write() syscall logic.
  * @q:		pointer to &struct vb2_queue with videobuf2 queue.
@@ -1103,8 +1103,8 @@ size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
  * @ppos:	file handle position tracking pointer
  * @nonblock:	mode selector (1 means blocking calls, 0 means nonblocking)
  */
-size_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
-		loff_t *ppos, int nonblock);
+ssize_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count,
+		  loff_t *ppos, int nonblock);
 
 /**
  * typedef vb2_thread_fnc - callback function for use with vb2_thread.
-- 
2.39.5
Re: [PATCH v3] media: vb2: use ssize_t for vb2_read/vb2_write
Posted by Zile Xiong 2 days, 22 hours ago
Hi,

A quick follow-up on this patch.

It looks like there may be an issue in the CI/patch parsing path. 
The CI output seemed to pick up a malformed/combined version of the patch, which led to some unexpected checks around the trailers. 
Also, v3 itself does not appear to have triggered CI.

v3 was intended to address the previous feedback:
- add Cc: stable@vger.kernel.org
- fix the formatting issues reported by checkpatch script

Could you please take a look when convenient? If needed, I can resend v3.

Thanks,
Zile