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:37:31 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request, #6663: fs/fs_sync/ioctl: update
Donny9 opened a new pull request, #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663
## Summary
fs/fs_sync: support file sync operation by ioctl with BIOC_FLUSH
fs/fs_ioctl: support get sector size by cmd BIOC_BLKSSZGET
## Impact
support BIOC_BLKSSZGET
## Testing
Vela CI
--
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 diff in pull request #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663#discussion_r926688162
##########
fs/vfs/fs_ioctl.c:
##########
@@ -110,6 +110,18 @@ int file_vioctl(FAR struct file *filep, int req, va_list ap)
ret = inode_getpath(inode, (FAR char *)(uintptr_t)arg);
}
break;
+
+ case BIOC_BLKSSZGET:
+ if (inode->u.i_ops != NULL && inode->u.i_ops->ioctl != NULL)
+ {
+ struct geometry geo;
+ ret = inode->u.i_ops->ioctl(filep, BIOC_GEOMETRY,
+ (unsigned long)&geo);
Review Comment:
Do we need
```suggestion
(unsigned long)(uintptr_t)&geo);
```
?
--
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] Donny9 commented on a diff in pull request #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663#discussion_r926710170
##########
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:
Done! Thank you.
##########
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:
inode->u.i_mops
--
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] hartmannathan commented on a diff in pull request #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663#discussion_r926710919
##########
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:
Done. Thank you~
##########
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:
Done. Thank you~
--
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] Donny9 commented on a diff in pull request #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663#discussion_r926698626
##########
fs/vfs/fs_ioctl.c:
##########
@@ -110,6 +110,18 @@ int file_vioctl(FAR struct file *filep, int req, va_list ap)
ret = inode_getpath(inode, (FAR char *)(uintptr_t)arg);
}
break;
+
+ case BIOC_BLKSSZGET:
+ if (inode->u.i_ops != NULL && inode->u.i_ops->ioctl != NULL)
+ {
+ struct geometry geo;
+ ret = inode->u.i_ops->ioctl(filep, BIOC_GEOMETRY,
+ (unsigned long)&geo);
Review Comment:
Done, thank you.
--
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 #6663: fs/fs_sync/ioctl: update
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #6663:
URL: https://github.com/apache/incubator-nuttx/pull/6663
--
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