You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/07/10 15:35:36 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #4116: net: Add file_socket function
xiaoxiang781216 opened a new pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116
## Summary
and move the socket special process from fstat/nx_vfcntl/ to file_fstat/file_vfcntl
## Impact
Make the behaviour is smae between fstat/fcntl and file_fstat/file_fcntl
## Testing
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671402821
##########
File path: net/socket/net_sendfile.c
##########
@@ -74,7 +74,7 @@
*
* Returned Value:
* On success, returns the number of characters sent. On error,
- * -1 is returned, and errno is set appropriately:
+ * the negative errno is return appropriately:
Review comment:
```suggestion
* the negative errno is returned appropriately:
```
nit: Typo
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671349747
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
Review comment:
```suggestion
/* Use file_seek to get the current file position */
```
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
Review comment:
```suggestion
/* Use file_seek again to set the new file position */
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671348765
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
+ * return zero.
+ */
+
+ if (nbyteswritten >= 0)
+ {
+ /* Advance the buffer pointer and decrement the number of
+ * bytes remaining in the iobuffer. Typically, nbytesread
+ * will now be zero.
+ */
+
+ wrbuffer += nbyteswritten;
+ nbytesread -= nbyteswritten;
+
+ /* Increment the total number of bytes successfully
+ * transferred.
+ */
+
+ ntransferred += nbyteswritten;
+ }
+
+ /* Otherwise an error occurred */
+
+ else
+ {
+ /* Check for a write ERROR. EINTR is a special case. This
+ * function should break out and return an error if EINTR
+ * is returned and no data has been transferred. But what
+ * should it do if some data has been transferred? I
+ * suppose just continue?
+ */
+
+ if (nbyteswritten != -EINTR || ntransferred == 0)
+ {
+ /* Write error. Break out and return the error
+ * condition.
+ */
+
+ ntransferred = nbyteswritten;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread > 0);
+ }
+ }
+
+ /* Release the I/O buffer */
+
+ kmm_free(iobuffer);
+
+ /* Return the current file position */
+
+ if (offset)
+ {
+ /* Use lseek to get the current file position */
Review comment:
```suggestion
/* Use file_seek to get the current file position */
```
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
+ * return zero.
+ */
+
+ if (nbyteswritten >= 0)
+ {
+ /* Advance the buffer pointer and decrement the number of
+ * bytes remaining in the iobuffer. Typically, nbytesread
+ * will now be zero.
+ */
+
+ wrbuffer += nbyteswritten;
+ nbytesread -= nbyteswritten;
+
+ /* Increment the total number of bytes successfully
+ * transferred.
+ */
+
+ ntransferred += nbyteswritten;
+ }
+
+ /* Otherwise an error occurred */
+
+ else
+ {
+ /* Check for a write ERROR. EINTR is a special case. This
+ * function should break out and return an error if EINTR
+ * is returned and no data has been transferred. But what
+ * should it do if some data has been transferred? I
+ * suppose just continue?
+ */
+
+ if (nbyteswritten != -EINTR || ntransferred == 0)
+ {
+ /* Write error. Break out and return the error
+ * condition.
+ */
+
+ ntransferred = nbyteswritten;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread > 0);
+ }
+ }
+
+ /* Release the I/O buffer */
+
+ kmm_free(iobuffer);
+
+ /* Return the current file position */
+
+ if (offset)
+ {
+ /* Use lseek to get the current file position */
+
+ off_t curpos = file_seek(infile, 0, SEEK_CUR);
+ if (curpos < 0)
+ {
+ return curpos;
+ }
+
+ /* Return the current file position */
+
+ *offset = curpos;
+
+ /* Use lseek again to restore the original file position */
Review comment:
```suggestion
/* Use file_seek again to restore the original file position */
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#issuecomment-881542185
I think overall it looks good.
I've seen some other PRs with the same approach of changing the file operations to use `filep` structure instead of file descriptors. Could you explain the motivation for this refactor?
My first impression it seems that it looks cleaner, but I'd like to have a better comprehension of this.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#issuecomment-881590274
> File descriptor always associate with a particular task, but kernel thread or function should be task independent as much as possible. This patch series try to limit file descriptor function as a thin wrapper around file structure function.
True, it makes sense.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#issuecomment-881523026
@gustavonihei could you review this PR?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#issuecomment-881577319
File descriptor always associate with a particular task, but kernel thread or function should be task independent as much as possible. This patch series try to limit file descriptor function as a thin wrapper around file structure function.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei edited a comment on pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#issuecomment-881590274
> File descriptor always associate with a particular task, but kernel thread or function should be task independent as much as possible. This patch series try to limit file descriptor function as a thin wrapper around file structure function.
True, it makes sense. Thanks for the explanation.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671342941
##########
File path: net/socket/net_sendfile.c
##########
@@ -130,8 +130,8 @@ ssize_t psock_sendfile(FAR struct socket *psock, FAR struct file *infile,
if (psock == NULL || psock->s_conn == NULL)
{
nerr("ERROR: Invalid socket\n");
- _SO_SETERRNO(psock, EBADF);
- return ERROR;
+ psock->s_error = EBADF;
Review comment:
`psock_sendfile` documentation must be updated to reflect the current implementation. The __Return Value__ section still states that on failure it returns -1 and sets `errno`.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei merged pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671385494
##########
File path: net/socket/net_sendfile.c
##########
@@ -130,8 +130,8 @@ ssize_t psock_sendfile(FAR struct socket *psock, FAR struct file *infile,
if (psock == NULL || psock->s_conn == NULL)
{
nerr("ERROR: Invalid socket\n");
- _SO_SETERRNO(psock, EBADF);
- return ERROR;
+ psock->s_error = EBADF;
Review comment:
Done.
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
+ * return zero.
+ */
+
+ if (nbyteswritten >= 0)
+ {
+ /* Advance the buffer pointer and decrement the number of
+ * bytes remaining in the iobuffer. Typically, nbytesread
+ * will now be zero.
+ */
+
+ wrbuffer += nbyteswritten;
+ nbytesread -= nbyteswritten;
+
+ /* Increment the total number of bytes successfully
+ * transferred.
+ */
+
+ ntransferred += nbyteswritten;
+ }
+
+ /* Otherwise an error occurred */
+
+ else
+ {
+ /* Check for a write ERROR. EINTR is a special case. This
+ * function should break out and return an error if EINTR
+ * is returned and no data has been transferred. But what
+ * should it do if some data has been transferred? I
+ * suppose just continue?
+ */
+
+ if (nbyteswritten != -EINTR || ntransferred == 0)
+ {
+ /* Write error. Break out and return the error
+ * condition.
+ */
+
+ ntransferred = nbyteswritten;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread > 0);
+ }
+ }
+
+ /* Release the I/O buffer */
+
+ kmm_free(iobuffer);
+
+ /* Return the current file position */
+
+ if (offset)
+ {
+ /* Use lseek to get the current file position */
Review comment:
Done.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671349207
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
Review comment:
```suggestion
/* Check for a complete (or partial) write. write() should not
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671385641
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
+ * return zero.
+ */
+
+ if (nbyteswritten >= 0)
+ {
+ /* Advance the buffer pointer and decrement the number of
+ * bytes remaining in the iobuffer. Typically, nbytesread
+ * will now be zero.
+ */
+
+ wrbuffer += nbyteswritten;
+ nbytesread -= nbyteswritten;
+
+ /* Increment the total number of bytes successfully
+ * transferred.
+ */
+
+ ntransferred += nbyteswritten;
+ }
+
+ /* Otherwise an error occurred */
+
+ else
+ {
+ /* Check for a write ERROR. EINTR is a special case. This
+ * function should break out and return an error if EINTR
+ * is returned and no data has been transferred. But what
+ * should it do if some data has been transferred? I
+ * suppose just continue?
+ */
+
+ if (nbyteswritten != -EINTR || ntransferred == 0)
+ {
+ /* Write error. Break out and return the error
+ * condition.
+ */
+
+ ntransferred = nbyteswritten;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread > 0);
+ }
+ }
+
+ /* Release the I/O buffer */
+
+ kmm_free(iobuffer);
+
+ /* Return the current file position */
+
+ if (offset)
+ {
+ /* Use lseek to get the current file position */
+
+ off_t curpos = file_seek(infile, 0, SEEK_CUR);
+ if (curpos < 0)
+ {
+ return curpos;
+ }
+
+ /* Return the current file position */
+
+ *offset = curpos;
+
+ /* Use lseek again to restore the original file position */
Review comment:
Done.
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
+
+ newpos = file_seek(infile, *offset, SEEK_SET);
+ if (newpos < 0)
+ {
+ return newpos;
+ }
+ }
+
+ /* Allocate an I/O buffer */
+
+ iobuffer = kmm_malloc(CONFIG_SENDFILE_BUFSIZE);
+ if (!iobuffer)
+ {
+ return -ENOMEM;
+ }
+
+ /* Now transfer 'count' bytes from the infile to the outfile */
+
+ for (ntransferred = 0, endxfr = false; ntransferred < count && !endxfr; )
+ {
+ /* Loop until the read side of the transfer comes to some conclusion */
+
+ do
+ {
+ /* Read a buffer of data from the infile */
+
+ nbytesread = count - ntransferred;
+ if (nbytesread > CONFIG_SENDFILE_BUFSIZE)
+ {
+ nbytesread = CONFIG_SENDFILE_BUFSIZE;
+ }
+
+ nbytesread = file_read(infile, iobuffer, nbytesread);
+
+ /* Check for end of file */
+
+ if (nbytesread == 0)
+ {
+ /* End of file. Break out and return current number of bytes
+ * transferred.
+ */
+
+ endxfr = true;
+ break;
+ }
+
+ /* Check for a read ERROR. EINTR is a special case. This function
+ * should break out and return an error if EINTR is returned and
+ * no data has been transferred. But what should it do if some
+ * data has been transferred? I suppose just continue?
+ */
+
+ else if (nbytesread < 0)
+ {
+ /* EINTR is not an error (but will still stop the copy) */
+
+ if (nbytesread != -EINTR || ntransferred == 0)
+ {
+ /* Read error. Break out and return the error condition. */
+
+ ntransferred = nbytesread;
+ endxfr = true;
+ break;
+ }
+ }
+ }
+ while (nbytesread < 0);
+
+ /* Was anything read? */
+
+ if (!endxfr)
+ {
+ /* Yes.. Loop until the read side of the transfer comes to some
+ * conclusion.
+ */
+
+ wrbuffer = iobuffer;
+ do
+ {
+ /* Write the buffer of data to the outfile */
+
+ nbyteswritten = file_write(outfile, wrbuffer, nbytesread);
+
+ /* Check for a complete (or parial) write. write() should not
Review comment:
Done.
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
Review comment:
Done.
##########
File path: fs/vfs/fs_sendfile.c
##########
@@ -26,20 +26,255 @@
#include <sys/sendfile.h>
#include <stdbool.h>
-#include <stdlib.h>
-#include <unistd.h>
#include <errno.h>
-#include <assert.h>
-#include <nuttx/sched.h>
+#include <nuttx/kmalloc.h>
#include <nuttx/net/net.h>
-#ifdef CONFIG_NET_SENDFILE
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static ssize_t copyfile(FAR struct file *outfile, FAR struct file *infile,
+ off_t *offset, size_t count)
+{
+ FAR uint8_t *iobuffer;
+ FAR uint8_t *wrbuffer;
+ off_t startpos = 0;
+ ssize_t nbytesread;
+ ssize_t nbyteswritten;
+ size_t ntransferred;
+ bool endxfr;
+
+ /* Get the current file position. */
+
+ if (offset)
+ {
+ off_t newpos;
+
+ /* Use lseek to get the current file position */
+
+ startpos = file_seek(infile, 0, SEEK_CUR);
+ if (startpos < 0)
+ {
+ return startpos;
+ }
+
+ /* Use lseek again to set the new file position */
Review comment:
Done.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4116: net: Add file_socket function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4116:
URL: https://github.com/apache/incubator-nuttx/pull/4116#discussion_r671613933
##########
File path: net/socket/net_sendfile.c
##########
@@ -74,7 +74,7 @@
*
* Returned Value:
* On success, returns the number of characters sent. On error,
- * -1 is returned, and errno is set appropriately:
+ * the negative errno is return appropriately:
Review comment:
Done, thanks.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org