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/01/14 05:20:36 UTC

[GitHub] [incubator-nuttx] yamt opened a new pull request #5224: Do not require write-access for fsync

yamt opened a new pull request #5224:
URL: https://github.com/apache/incubator-nuttx/pull/5224


   ## Summary
   
   Do not require write-access for fsync
   
   * fsync doesn't modify the file. It doesn't make sense to require
     write-access.
   
   * This matches what ~all other systems do.
     At least Linux, macOS, and NetBSD.
   
   ## Impact
   
   ## 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] pkarashchenko merged pull request #5224: Do not require write-access for fsync

Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5224:
URL: https://github.com/apache/incubator-nuttx/pull/5224


   


-- 
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] pkarashchenko commented on a change in pull request #5224: Do not require write-access for fsync

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5224:
URL: https://github.com/apache/incubator-nuttx/pull/5224#discussion_r784626789



##########
File path: fs/vfs/fs_fsync.c
##########
@@ -55,13 +55,6 @@ int file_fsync(FAR struct file *filep)
 {
   struct inode *inode;
 
-  /* Was this file opened for write access? */
-
-  if ((filep->f_oflags & O_WROK) == 0)

Review comment:
       Yes. I missed that. Thanks for clarification!




-- 
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] yamt commented on a change in pull request #5224: Do not require write-access for fsync

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #5224:
URL: https://github.com/apache/incubator-nuttx/pull/5224#discussion_r784624389



##########
File path: fs/vfs/fs_fsync.c
##########
@@ -55,13 +55,6 @@ int file_fsync(FAR struct file *filep)
 {
   struct inode *inode;
 
-  /* Was this file opened for write access? */
-
-  if ((filep->f_oflags & O_WROK) == 0)

Review comment:
       eg. you can make modifications to the file using another open descriptor.
   




-- 
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] pkarashchenko commented on a change in pull request #5224: Do not require write-access for fsync

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5224:
URL: https://github.com/apache/incubator-nuttx/pull/5224#discussion_r784620923



##########
File path: fs/vfs/fs_fsync.c
##########
@@ -55,13 +55,6 @@ int file_fsync(FAR struct file *filep)
 {
   struct inode *inode;
 
-  /* Was this file opened for write access? */
-
-  if ((filep->f_oflags & O_WROK) == 0)

Review comment:
       What is the point of `fsync` in case if file is not open for write? Maybe reasonable just return `OK` in this case instead of removing check? I mean that we really do not need to calll `inode->​u​.​i_mops​->​sync​` if file is not open for writing.




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