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 2022/07/21 13:51:57 UTC

[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6663: fs/fs_sync/ioctl: update

hartmannathan commented on code in PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663#discussion_r926700540


##########
fs/vfs/fs_fsync.c:
##########
@@ -53,23 +54,31 @@
 
 int file_fsync(FAR struct file *filep)
 {
-  struct inode *inode;
+  FAR struct inode *inode;
+  int ret;
 
   /* Is this inode a registered mountpoint? Does it support the
    * sync operations may be relevant to device drivers but only
    * the mountpoint operations vtable contains a sync method.
    */
 
   inode = filep->f_inode;
-  if (!inode || !INODE_IS_MOUNTPT(inode) ||
-      !inode->u.i_mops || !inode->u.i_mops->sync)
+  if (inode != NULL)
     {
-      return -EINVAL;
+      if (INODE_IS_MOUNTPT(inode) && inode->u.i_mops->sync)

Review Comment:
   Before checking `inode->u.i_mops->sync` I think we need to check `inode->u.i_mops` is not NULL, else sync may be in a NULL or undefined location.



##########
fs/vfs/fs_fsync.c:
##########
@@ -53,23 +54,31 @@
 
 int file_fsync(FAR struct file *filep)
 {
-  struct inode *inode;
+  FAR struct inode *inode;
+  int ret;
 
   /* Is this inode a registered mountpoint? Does it support the
    * sync operations may be relevant to device drivers but only
    * the mountpoint operations vtable contains a sync method.
    */
 
   inode = filep->f_inode;
-  if (!inode || !INODE_IS_MOUNTPT(inode) ||
-      !inode->u.i_mops || !inode->u.i_mops->sync)
+  if (inode != NULL)
     {
-      return -EINVAL;
+      if (INODE_IS_MOUNTPT(inode) && inode->u.i_mops->sync)
+        {
+          /* Yes, then tell the mountpoint to sync this file */
+
+          return inode->u.i_mops->sync(filep);
+        }
+      else if (inode->u.i_ops->ioctl)

Review Comment:
   Before checking `inode->u.i_ops->ioctl` I think we need to check `inode->u.i_ops` is not NULL, for same reason as above.



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