xiaoxiang781216 commented on code in PR #16361:
URL: https://github.com/apache/nuttx/pull/16361#discussion_r2085116249


##########
net/local/local_recvmsg.c:
##########
@@ -175,7 +175,7 @@ static void local_recvctl(FAR struct local_conn_s *conn,
   count = peer->lc_cfpcount;
   for (i = 0; i < count; i++)
     {
-      fds[i] = file_dup(peer->lc_cfps[i], 0,
+      fds[i] = file_dup(peer->lc_cfps[i], 0, /* TODO: WHat about this ? */

Review Comment:
   ditto



##########
fs/vfs/fs_dup.c:
##########
@@ -73,21 +73,28 @@ int file_dup(FAR struct file *filep, int minfd, int flags)
 
   ret = fs_getfilep(fd2, &filep2);
 #ifdef CONFIG_FDSAN
-  f_tag_fdsan = filep2->f_tag_fdsan;
+  f_tag_fdsan = fs_fdcntl(fd2, FIOC_GETTAG_FDSAN, 0);
 #endif
 
 #ifdef CONFIG_FDCHECK
-  f_tag_fdcheck = filep2->f_tag_fdcheck;
+  f_tag_fdcheck = fs_fdcntl(fd2, FIOC_GETTAG_FDCHECK, 0);
 #endif
   DEBUGASSERT(ret >= 0);
 
-  ret = file_dup3(filep, filep2, flags);
+  ret = file_dup2(filep, filep2);
+  if (ret >= 0 && flags != 0)

Review Comment:
   (flags & O_CLOEXEC)



##########
fs/vfs/fs_dup3.c:
##########
@@ -0,0 +1,196 @@
+/****************************************************************************
+ * fs/vfs/fs_dup2.c

Review Comment:
   fs_dup3.c



##########
include/nuttx/fs/fs.h:
##########
@@ -959,7 +964,7 @@ int files_duplist(FAR struct filelist *plist, FAR struct 
filelist *clist,
  *
  ****************************************************************************/
 
-FAR struct file *files_fget(FAR struct filelist *list, int fd);
+FAR struct file *files_fget(FAR struct fdlist *list, int fd);

Review Comment:
   fdlist_getfile



##########
include/nuttx/fs/fs.h:
##########
@@ -1228,29 +1215,31 @@ int fs_putfilep(FAR struct file *filep);
 #endif
 
 /****************************************************************************
- * Name: file_close
+ * Name: fs_fdcntl
  *
  * Description:
- *   Close a file that was previously opened with file_open().
+ *   fs_fdcntl() will perform the operation specified by 'cmd' on an open
+ *   file.
  *
  * Input Parameters:
- *   filep - A pointer to a user provided memory location containing the
- *           open file data returned by file_open().
+ *   fd  - File descriptor of the open file
+ *   cmd - Identifies the operation to be performed.  Command specific
+ *         arguments may follow.
+ *   arg - The argument.
  *
  * Returned Value:
- *   Zero (OK) is returned on success; A negated errno value is returned on
- *   any failure to indicate the nature of the failure.
+ *   A non-negative value is returned on success; a negated errno value is
+ *   returned on any failure.
  *
  ****************************************************************************/
 
-int file_close(FAR struct file *filep);
+int fs_fdcntl(int fd, int cmd, unsigned long arg);

Review Comment:
   nx_fcntl?



##########
include/nuttx/fs/fs.h:
##########
@@ -474,10 +483,6 @@ struct file
   uint8_t           f_tag_fdcheck; /* File owner fdcheck tag, init to 0 */
 #endif
 
-#if CONFIG_FS_BACKTRACE > 0
-  FAR void         *f_backtrace[CONFIG_FS_BACKTRACE]; /* Backtrace to while 
file opens */
-#endif
-
 #if CONFIG_FS_LOCK_BUCKET_SIZE > 0
   bool              locked; /* Filelock state: false - unlocked, true - locked 
*/

Review Comment:
   need move to `struct file`



##########
tools/pynuttx/nxgdb/protocols/fs.py:
##########
@@ -31,10 +31,7 @@ class File(Value):
     f_pos: Value
     f_inode: Value
     f_priv: Value
-    f_tag_fdsan: Value
-    f_tag_fdcheck: Value
     f_backtrace: Value
-    locked: Value

Review Comment:
   need keep loccked



##########
fs/vfs/fs_dup3.c:
##########
@@ -1,5 +1,5 @@
 /****************************************************************************
- * fs/vfs/fs_dup2.c
+ * fs/vfs/fs_dup3.c

Review Comment:
   move to previous line



##########
include/nuttx/fs/fs.h:
##########
@@ -889,7 +894,7 @@ int nx_umount2(FAR const char *target, unsigned int flags);
  *
  ****************************************************************************/
 
-void files_initlist(FAR struct filelist *list);
+void files_initlist(FAR struct fdlist *list);

Review Comment:
   should we change files_xxxlist to fdlist_xxx



##########
net/local/local_sendmsg.c:
##########
@@ -125,7 +125,7 @@ static int local_sendctl(FAR struct local_conn_s *conn,
               goto fail;
             }
 
-          ret = file_dup2(filep, filep2);
+          ret = file_dup2(filep, filep2); /* TODO: What about this ? */

Review Comment:
   what's problem here to add this comment?



##########
include/nuttx/fs/fs.h:
##########
@@ -474,10 +483,6 @@ struct file
   uint8_t           f_tag_fdcheck; /* File owner fdcheck tag, init to 0 */
 #endif
 
-#if CONFIG_FS_BACKTRACE > 0
-  FAR void         *f_backtrace[CONFIG_FS_BACKTRACE]; /* Backtrace to while 
file opens */

Review Comment:
   need keep  in `struct fd`



##########
include/nuttx/fs/fs.h:
##########
@@ -466,6 +466,15 @@ struct file
   off_t             f_pos;      /* File position */

Review Comment:
   should we drop CONFIG_FS_REFCOUNT as you mention in pr



##########
fs/vfs/fs_dup.c:
##########
@@ -73,21 +73,28 @@ int file_dup(FAR struct file *filep, int minfd, int flags)
 
   ret = fs_getfilep(fd2, &filep2);
 #ifdef CONFIG_FDSAN
-  f_tag_fdsan = filep2->f_tag_fdsan;
+  f_tag_fdsan = fs_fdcntl(fd2, FIOC_GETTAG_FDSAN, 0);
 #endif
 
 #ifdef CONFIG_FDCHECK
-  f_tag_fdcheck = filep2->f_tag_fdcheck;
+  f_tag_fdcheck = fs_fdcntl(fd2, FIOC_GETTAG_FDCHECK, 0);
 #endif
   DEBUGASSERT(ret >= 0);
 
-  ret = file_dup3(filep, filep2, flags);
+  ret = file_dup2(filep, filep2);
+  if (ret >= 0 && flags != 0)
+    {
+      /* Set close on exec flag */
+
+      ret = fs_fdcntl(fd2, F_SETFD, flags == O_CLOEXEC ? FD_CLOEXEC : 0);

Review Comment:
   change `==` to `&`



##########
include/nuttx/sched.h:
##########
@@ -539,7 +539,7 @@ struct task_group_s
 
   /* File descriptors *******************************************************/
 
-  struct filelist tg_filelist;      /* Maps file descriptor to file         */
+  struct fdlist tg_filelist;        /* Maps file descriptor to file         */

Review Comment:
   should we change tg_filelist to tg_fdlist



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

Reply via email to