xiaoxiang781216 commented on code in PR #16499:
URL: https://github.com/apache/nuttx/pull/16499#discussion_r2135918282
##########
fs/vfs/fs_fcntl.c:
##########
@@ -347,27 +283,102 @@ int fcntl(int fd, int cmd, ...)
va_start(ap, cmd);
- /* Get the file structure corresponding to the file descriptor. */
-
ret = file_get(fd, &filep);
- if (ret >= 0)
+ if (ret < 0)
{
- /* Let file_vfcntl() do the real work. The errno is not set on
- * failures.
- */
-
- ret = file_vfcntl(filep, cmd, ap);
- file_put(filep);
+ goto errout;
}
- if (ret < 0)
+ switch (cmd)
{
- set_errno(-ret);
- ret = ERROR;
+ case F_DUPFD:
+ /* Return a new file descriptor which shall be the lowest numbered
+ * available (that is, not already open) file descriptor greater than
+ * or equal to the third argument, arg, taken as an integer of type
+ * int. The new file descriptor shall refer to the same open file
+ * description as the original file descriptor, and shall share any
+ * locks. The FD_CLOEXEC flag associated with the new file
+ * descriptor shall be cleared to keep the file open across calls to
+ * one of the exec functions.
+ */
+
+ {
+ ret = file_dup(filep, va_arg(ap, int), 0);
+ }
+ break;
+
+ case F_DUPFD_CLOEXEC:
+ {
+ ret = file_dup(filep, va_arg(ap, int), O_CLOEXEC);
+ }
+ break;
+
+ case F_GETFD:
+ /* Get the file descriptor flags defined in <fcntl.h> that are
+ * associated with the file descriptor fd. File descriptor flags are
+ * associated with a single file descriptor and do not affect other
+ * file descriptors that refer to the same file.
+ */
+
+ {
+ int flags;
+
+ ret = file_ioctl(filep, FIOGCLEX, &flags);
+ if (ret >= 0)
+ {
+ ret = flags;
+ }
+ }
+ break;
+
+ case F_SETFD:
+ /* Set the file descriptor flags defined in <fcntl.h>, that are
+ * associated with fd, to the third argument, arg, taken as type int.
+ * If the FD_CLOEXEC flag in the third argument is 0, the file shall
+ * remain open across the exec functions; otherwise, the file shall
+ * be closed upon successful execution of one of the exec functions.
+ */
+
+ {
+ int oflags = va_arg(ap, int);
+
+ if (oflags & ~FD_CLOEXEC)
+ {
+ file_put(filep);
+ ret = -ENOSYS;
+ goto errout;
+ }
+
+ if (oflags & FD_CLOEXEC)
+ {
+ ret = file_ioctl(filep, FIOCLEX, NULL);
+ }
+ else
+ {
+ ret = file_ioctl(filep, FIONCLEX, NULL);
+ }
+ }
+ break;
+
+ default:
+ {
+ ret = file_vfcntl(filep, cmd, ap);
+ }
+
Review Comment:
remove the blank line
##########
fs/vfs/fs_fcntl.c:
##########
@@ -347,27 +283,102 @@ int fcntl(int fd, int cmd, ...)
va_start(ap, cmd);
- /* Get the file structure corresponding to the file descriptor. */
-
ret = file_get(fd, &filep);
- if (ret >= 0)
+ if (ret < 0)
{
- /* Let file_vfcntl() do the real work. The errno is not set on
- * failures.
- */
-
- ret = file_vfcntl(filep, cmd, ap);
- file_put(filep);
+ goto errout;
}
- if (ret < 0)
+ switch (cmd)
{
- set_errno(-ret);
- ret = ERROR;
+ case F_DUPFD:
+ /* Return a new file descriptor which shall be the lowest numbered
+ * available (that is, not already open) file descriptor greater than
+ * or equal to the third argument, arg, taken as an integer of type
+ * int. The new file descriptor shall refer to the same open file
+ * description as the original file descriptor, and shall share any
+ * locks. The FD_CLOEXEC flag associated with the new file
+ * descriptor shall be cleared to keep the file open across calls to
+ * one of the exec functions.
+ */
+
+ {
+ ret = file_dup(filep, va_arg(ap, int), 0);
+ }
+ break;
+
+ case F_DUPFD_CLOEXEC:
+ {
+ ret = file_dup(filep, va_arg(ap, int), O_CLOEXEC);
+ }
+ break;
+
+ case F_GETFD:
+ /* Get the file descriptor flags defined in <fcntl.h> that are
+ * associated with the file descriptor fd. File descriptor flags are
+ * associated with a single file descriptor and do not affect other
+ * file descriptors that refer to the same file.
+ */
+
+ {
+ int flags;
+
+ ret = file_ioctl(filep, FIOGCLEX, &flags);
+ if (ret >= 0)
+ {
+ ret = flags;
+ }
+ }
+ break;
+
+ case F_SETFD:
+ /* Set the file descriptor flags defined in <fcntl.h>, that are
+ * associated with fd, to the third argument, arg, taken as type int.
+ * If the FD_CLOEXEC flag in the third argument is 0, the file shall
+ * remain open across the exec functions; otherwise, the file shall
+ * be closed upon successful execution of one of the exec functions.
+ */
+
+ {
+ int oflags = va_arg(ap, int);
+
+ if (oflags & ~FD_CLOEXEC)
+ {
+ file_put(filep);
+ ret = -ENOSYS;
+ goto errout;
Review Comment:
break and remove line 347
##########
fs/inode/fs_files.c:
##########
@@ -676,91 +789,68 @@ int files_duplist(FAR struct filelist *plist, FAR struct
filelist *clist,
continue;
}
- ret = files_extend(clist, i + 1);
+ ret = fdlist_extend(clist, i + 1);
if (ret < 0)
{
file_put(filep);
return ret;
}
- /* Yes... duplicate it for the child, include O_CLOEXEC flag. */
+ /* Assign filep to the child's descriptor list. Omit the flags */
- filep2 = files_fget_by_index(clist, i, j, &new);
- ret = file_dup2(filep, filep2);
- file_put(filep2);
+ fdlist_install(clist, fd, filep, 0);
file_put(filep);
- if (ret < 0)
- {
- if (new)
- {
- file_put(filep2);
- }
-
- return ret;
- }
}
}
return OK;
}
/****************************************************************************
- * Name: file_get
+ * Name: file_get2
*
* Description:
* Given a file descriptor, return the corresponding instance of struct
- * file.
+ * fd and filep
*
* Input Parameters:
* fd - The file descriptor
* filep - The location to return the struct file instance
+ * fdp - The location to return the struct fd instance
*
* Returned Value:
* Zero (OK) is returned on success; a negated errno value is returned on
* any failure.
*
****************************************************************************/
-int file_get(int fd, FAR struct file **filep)
+int file_get2(int fd, FAR struct file **filep, FAR struct fd **fdp)
{
- FAR struct filelist *list;
-
-#ifdef CONFIG_FDCHECK
- fd = fdcheck_restore(fd);
-#endif
-
- *filep = NULL;
+ FAR struct fdlist *list;
+ FAR struct file *filep1;
+ FAR struct fd *fdp1;
+ int ret;
- list = nxsched_get_files();
+ list = nxsched_get_fdlist();
- /* The file list can be NULL under two cases: (1) One is an obscure
- * cornercase: When memory management debug output is enabled. Then
- * there may be attempts to write to stdout from malloc before the group
- * data has been allocated. The other other is (2) if this is a kernel
- * thread. Kernel threads have no allocated file descriptors.
+ /* The descriptor is in a valid range to file descriptor... Get the
+ * thread-specific file list.
*/
- if (list == NULL)
+ ret = fdlist_get2(list, fd, &filep1, &fdp1);
Review Comment:
remove filep1 and fdp1, use filep and fdp directly
##########
fs/inode/fs_files.c:
##########
@@ -587,37 +618,120 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR
struct inode *inode,
#endif
}
+/****************************************************************************
+ * Name: fdlist_allocate
+ *
+ * Description:
+ * Allocate a struct fd instance and associate it with an empty file
+ * instance. The difference between this function and
+ * file_allocate_from_inode is that this function is only used for
+ * placeholder purposes. Later, the caller will initialize the file entity
+ * through the returned filep.
+ *
+ * The fd allocated by this function can be released using fdlist_close.
+ *
+ * Returned Value:
+ * Returns the file descriptor == index into the files array on success;
+ * a negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int fdlist_allocate(FAR struct fdlist *list, int oflags,
+ int minfd, FAR struct file **filep)
+{
+ int fd;
+
+ *filep = fs_heap_zalloc(sizeof(struct file));
+ if (*filep == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ fd = fdlist_dupfile(list, oflags, minfd, *filep);
+ if (fd < 0)
+ {
+ fs_heap_free(*filep);
+ }
+
+ return fd;
+}
+
/****************************************************************************
* Name: file_allocate
*
* Description:
- * Allocate a struct files instance and associate it with an inode
- * instance.
+ * Allocate a struct fd instance and associate it with an empty file
+ * instance. The difference between this function and
+ * file_allocate_from_inode is that this function is only used for
+ * placeholder purposes. Later, the caller will initialize the file entity
+ * through the returned filep.
+ *
+ * The fd allocated by this function can be released using nx_close.
+ *
+ * Returned Value:
+ * Returns the file descriptor == index into the files array on success;
+ * a negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int file_allocate(int oflags, int minfd, FAR struct file **filep)
+{
+ return fdlist_allocate(nxsched_get_fdlist_from_tcb(this_task()), oflags,
Review Comment:
move oflags to next line
##########
fs/inode/fs_files.c:
##########
@@ -258,70 +292,43 @@ static void task_fssync(FAR struct tcb_s *tcb, FAR void
*arg)
*
****************************************************************************/
-int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2, int flags)
+int fdlist_dup3(FAR struct fdlist *list, int fd1, int fd2, int flags)
{
- FAR struct filelist *list;
FAR struct file *filep1;
- FAR struct file *filep;
- bool new = false;
- int count;
int ret;
if (fd1 == fd2)
{
- return fd1;
+ return -EINVAL;
}
#ifdef CONFIG_FDCHECK
- fd1 = fdcheck_restore(fd1);
fd2 = fdcheck_restore(fd2);
#endif
/* Get the file descriptor list. It should not be NULL in this context. */
- list = nxsched_get_files_from_tcb(tcb);
- count = files_countlist(list);
-
- if (fd1 < 0 || fd1 >= count || fd2 < 0)
+ if (fd2 < 0)
{
return -EBADF;
}
- if (fd2 >= count)
- {
- ret = files_extend(list, fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + 1);
- if (ret < 0)
- {
- return ret;
- }
- }
-
- filep1 = files_fget(list, fd1);
- if (filep1 == NULL)
+ ret = fdlist_extend(list,
Review Comment:
why remove the check at line 290
##########
fs/inode/fs_files.c:
##########
@@ -839,20 +934,39 @@ int file_put(FAR struct file *filep)
*
****************************************************************************/
-int nx_dup2_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2)
+int fdlist_dup2(FAR struct fdlist *list, int fd1, int fd2)
{
- return nx_dup3_from_tcb(tcb, fd1, fd2, 0);
+ /* If fd1 is a valid file descriptor, and fd1 == fd2, then dup2() does
+ * nothing, and returns fd2.
+ */
+
+ if (fd1 == fd2)
+ {
+ FAR struct file *filep;
+
+ if (file_get(fd1, &filep) < 0)
Review Comment:
```
int ret = file_get(fd1, &filep);
if (ret < 0)
{
return ret;
}
```
##########
fs/inode/fs_files.c:
##########
@@ -445,90 +454,120 @@ void files_putlist(FAR struct filelist *list)
{
for (j = CONFIG_NFILE_DESCRIPTORS_PER_BLOCK - 1; j >= 0; j--)
{
- file_close(&list->fl_files[i][j]);
+ fdlist_close(list, i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j);
}
if (i != 0)
{
- fs_heap_free(list->fl_files[i]);
+ fs_heap_free(list->fl_fds[i]);
}
}
- if (list->fl_files != &list->fl_prefile)
+ if (list->fl_fds != &list->fl_prefd)
{
- fs_heap_free(list->fl_files);
+ fs_heap_free(list->fl_fds);
}
}
/****************************************************************************
- * Name: files_countlist
+ * Name: fdlist_count
*
* Description:
- * Get file count from file list.
+ * Get file descriptor count from file list.
*
* Input Parameters:
- * list - Pointer to the file list structure.
+ * list - Pointer to the file descriptor list structure.
*
* Returned Value:
* file count of file list.
*
****************************************************************************/
-int files_countlist(FAR struct filelist *list)
+int fdlist_count(FAR struct fdlist *list)
{
return list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK;
}
/****************************************************************************
- * Name: files_fget
+ * Name: fdlist_get2
*
* Description:
- * Get the instance of struct file from file list by file descriptor.
+ * Given a file descriptor, return the corresponding instance of struct
+ * fd and filep.
*
* Input Parameters:
- * list - The list of files for a task.
- * fd - A valid descriptor between 0 and files_countlist(list).
+ * list - Pointer to the file descriptor list structure.
+ * fd - The file descriptor
+ * filep - The location to return the struct file instance
+ * fdp - The location to return the struct fd instance
*
* Returned Value:
- * Pointer to file structure of list[fd].
+ * Return the pointer to file structure of list[fd] when list[fd].f_file
+ * is valid, otherwise, a null pointer is returned.
*
****************************************************************************/
-FAR struct file *files_fget(FAR struct filelist *list, int fd)
+int fdlist_get2(FAR struct fdlist *list, int fd,
+ FAR struct file **filep, FAR struct fd **fdp)
{
- return files_fget_by_index(list, fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK,
- fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK, NULL);
+ if (list == NULL)
+ {
+ return -EAGAIN;
Review Comment:
-EINVAL
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
