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