xiaoxiang781216 commented on code in PR #13498:
URL: https://github.com/apache/nuttx/pull/13498#discussion_r1817692945
##########
drivers/misc/dev_zero.c:
##########
@@ -70,26 +73,39 @@ static const struct file_operations g_devzero_fops =
* Name: devzero_read
****************************************************************************/
-static ssize_t devzero_read(FAR struct file *filep, FAR char *buffer,
- size_t len)
+static ssize_t devzero_readv(FAR struct file *filep,
+ FAR const struct uio *uio)
{
+ ssize_t total = iovec_total_len(uio);
Review Comment:
should we accumulate the size in the for loop to avoid looping iov twice?
```suggestion
ssize_t total = 0;
```
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
+ FAR const struct uio *uio)
+{
+ const struct iovec *iov = uio->uio_iov;
Review Comment:
```suggestion
FAR const struct iovec *iov = uio->uio_iov;
```
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
+ FAR const struct uio *uio)
+{
+ const struct iovec *iov = uio->uio_iov;
+ int iovcnt = uio->uio_iovcnt;
+ FAR struct inode *inode = filep->f_inode;
+ ssize_t ntotal;
Review Comment:
```suggestion
ssize_t ntotal = 0;
```
##########
include/nuttx/fs/uio.h:
##########
@@ -0,0 +1,65 @@
+/****************************************************************************
+ * include/nuttx/fs/uio.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_FS_UIO_H
+#define __INCLUDE_NUTTX_FS_UIO_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Type Definitions
+ ****************************************************************************/
+
+/* The structure to describe an user I/O operation.
+ *
+ * At this point, this is a bare minimum for readv/writev.
+ * In the future, we might extend this for other things like
+ * the file offset for pread/pwrite.
+ *
+ * This structure was inspired by BSDs.
+ * (Thus it doesn't have the NuttX-style "_s" suffix.)
+ */
+
+struct uio
+{
+ FAR const struct iovec *uio_iov;
+ int uio_iovcnt;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_total_len
+ *
+ * Description:
+ * Return the total length of data in bytes.
+ * Or -EOVERFLOW.
+ *
+ ****************************************************************************/
+
+ssize_t iovec_total_len(FAR const struct uio *uio);
Review Comment:
```suggestion
ssize_t uio_total_len(FAR const struct uio *uio);
```
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
+ FAR const struct uio *uio)
+{
+ const struct iovec *iov = uio->uio_iov;
+ int iovcnt = uio->uio_iovcnt;
+ FAR struct inode *inode = filep->f_inode;
+ ssize_t ntotal;
+ ssize_t nwritten;
+ size_t remaining;
+ FAR uint8_t *buffer;
+ int i;
+
+ DEBUGASSERT(inode->u.i_ops->write != NULL);
+
+ /* Process each entry in the struct iovec array */
+
+ for (i = 0, ntotal = 0; i < iovcnt; i++)
+ {
+ /* Ignore zero-length writes */
+
+ if (iov[i].iov_len > 0)
+ {
+ buffer = iov[i].iov_base;
+ remaining = iov[i].iov_len;
+
+ /* Write repeatedly as necessary to write the entire buffer */
+
+ do
+ {
+ nwritten = inode->u.i_ops->write(filep, (void *)buffer,
Review Comment:
```suggestion
ssize_t nwritten = inode->u.i_ops->write(filep, buffer,
```
and remove line 59
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
+ FAR const struct uio *uio)
+{
+ const struct iovec *iov = uio->uio_iov;
+ int iovcnt = uio->uio_iovcnt;
+ FAR struct inode *inode = filep->f_inode;
+ ssize_t ntotal;
+ ssize_t nwritten;
+ size_t remaining;
+ FAR uint8_t *buffer;
+ int i;
+
+ DEBUGASSERT(inode->u.i_ops->write != NULL);
+
+ /* Process each entry in the struct iovec array */
+
+ for (i = 0, ntotal = 0; i < iovcnt; i++)
Review Comment:
```suggestion
for (i = 0; i < iovcnt; i++)
```
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
+ FAR const struct uio *uio)
+{
+ const struct iovec *iov = uio->uio_iov;
+ int iovcnt = uio->uio_iovcnt;
+ FAR struct inode *inode = filep->f_inode;
+ ssize_t ntotal;
+ ssize_t nwritten;
+ size_t remaining;
+ FAR uint8_t *buffer;
+ int i;
+
+ DEBUGASSERT(inode->u.i_ops->write != NULL);
+
+ /* Process each entry in the struct iovec array */
+
+ for (i = 0, ntotal = 0; i < iovcnt; i++)
+ {
+ /* Ignore zero-length writes */
+
+ if (iov[i].iov_len > 0)
Review Comment:
can we change to:
```suggestion
FAR uint8_t *buffer = iov[i].iov_base;
size_t remaining = iov[i].iov_len;
while (remaining > 0)
```
merge line 82 to line 81 and remove line 60-61.
##########
fs/vfs/fs_write.c:
##########
@@ -32,30 +32,96 @@
#include <assert.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_writev
+ *
+ * Description:
+ * Emulate writev using file_operation::write.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_writev(FAR struct file *filep,
Review Comment:
```suggestion
static ssize_t file_writev_compat(FAR struct file *filep,
```
##########
fs/vfs/fs_write.c:
##########
@@ -124,16 +233,12 @@ ssize_t file_write(FAR struct file *filep, FAR const void
*buf,
*
****************************************************************************/
-ssize_t nx_write(int fd, FAR const void *buf, size_t nbytes)
+ssize_t nx_writev(int fd, FAR const struct iovec *iov, int iovcnt)
{
+ struct uio uio;
Review Comment:
```suggestion
FAR struct file *filep;
struct uio uio;
```
##########
drivers/misc/dev_zero.c:
##########
@@ -70,26 +73,39 @@ static const struct file_operations g_devzero_fops =
* Name: devzero_read
****************************************************************************/
-static ssize_t devzero_read(FAR struct file *filep, FAR char *buffer,
- size_t len)
+static ssize_t devzero_readv(FAR struct file *filep,
+ FAR const struct uio *uio)
{
+ ssize_t total = iovec_total_len(uio);
+ int i;
+
UNUSED(filep);
- memset(buffer, 0, len);
- return len;
+ if (total < 0)
+ {
+ return total;
+ }
+
+ FAR const struct iovec *iov = uio->uio_iov;
Review Comment:
should move before line 82
##########
include/nuttx/fs/fs.h:
##########
@@ -183,6 +185,7 @@ struct statfs;
struct pollfd;
struct mtd_dev_s;
struct tcb_s;
+struct uio;
Review Comment:
should we move `struct uio` definition and `iovec_total_len` prototype to
include/nuttx/fs/fs.h and remove include/nuttx/fs/uio.h?
##########
fs/vfs/fs_read.c:
##########
@@ -32,37 +32,110 @@
#include <errno.h>
#include <nuttx/cancelpt.h>
+#include <nuttx/fs/uio.h>
#include "notify/notify.h"
#include "inode/inode.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iovec_compat_readv
+ *
+ * Description:
+ * Emulate readv using file_operation::read.
+ *
+ ****************************************************************************/
+
+static ssize_t iovec_compat_readv(FAR struct file *filep,
Review Comment:
```suggestion
static ssize_t file_writev_compat(FAR struct file *filep,
```
let's apply the similar suggestion from fs_write.c
##########
fs/vfs/fs_read.c:
##########
@@ -108,28 +184,63 @@ ssize_t file_read(FAR struct file *filep, FAR void *buf,
size_t nbytes)
}
/****************************************************************************
- * Name: nx_read
+ * Name: file_read
*
* Description:
- * nx_read() is an internal OS interface. It is functionally similar to
+ * file_read() is an internal OS interface. It is functionally similar to
* the standard read() interface except:
*
+ * - It does not modify the errno variable,
+ * - It is not a cancellation point,
+ * - It accepts a file structure instance instead of file descriptor.
+ *
+ * Input Parameters:
+ * filep - File structure instance
+ * buf - User-provided to save the data
+ * nbytes - The maximum size of the user-provided buffer
+ *
+ * Returned Value:
+ * The positive non-zero number of bytes read on success, 0 on if an
+ * end-of-file condition, or a negated errno value on any failure.
+ *
+ ****************************************************************************/
+
+ssize_t file_read(FAR struct file *filep, FAR void *buf, size_t nbytes)
+{
+ struct iovec iov;
+ struct uio uio;
+
+ iov.iov_base = buf;
+ iov.iov_len = nbytes;
+ uio.uio_iov = &iov;
+ uio.uio_iovcnt = 1;
+ return file_readv(filep, &uio);
+}
+
+/****************************************************************************
+ * Name: nx_readv
+ *
+ * Description:
+ * nx_readv() is an internal OS interface. It is functionally similar to
+ * the standard readv() interface except:
+ *
* - It does not modify the errno variable, and
* - It is not a cancellation point.
*
* Input Parameters:
* fd - File descriptor to read from
- * buf - User-provided to save the data
- * nbytes - The maximum size of the user-provided buffer
+ * iov - User-provided iovec to save the data
+ * iovcnt - The number of iovec
*
* Returned Value:
* The positive non-zero number of bytes read on success, 0 on if an
* end-of-file condition, or a negated errno value on any failure.
*
****************************************************************************/
-ssize_t nx_read(int fd, FAR void *buf, size_t nbytes)
+ssize_t nx_readv(int fd, FAR const struct iovec *iov, int iovcnt)
{
+ struct uio uio;
Review Comment:
```suggestion
FAR struct file *filep;
struct uio uio;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
