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 2020/07/03 07:43:51 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
xiaoxiang781216 opened a new pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355
## Summary
specified here:
https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/statvfs.h.html
## Impact
No, just change the struct and function name
## 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656648108
> @xiaoxiang781216 i think f_fsid is meant to be an ID for each individual file system, not file system type. how do you think?
Yes, you are right. After more thinking, I will keep both interfaces to improve the compatibility.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r449765278
##########
File path: include/sys/statvfs.h
##########
@@ -49,7 +49,14 @@
* Pre-processor Definitions
****************************************************************************/
-/* struct statfs file system types. */
+/* Definitions for the flag in `f_flag'. These definitions should be
+ * kept in sync with the definitions in <sys/mount.h>.
+ */
+
+#define ST_RDONLY 0x0001 /* Mount read-only. */
+#define ST_NOSUID 0x0002 /* Ignore suid and sgid bits. */
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r449730036
##########
File path: arch/sim/src/sim/up_hostfs.c
##########
@@ -432,32 +432,35 @@ int host_closedir(void *dirp)
}
/****************************************************************************
- * Name: host_statfs
+ * Name: host_statvfs
****************************************************************************/
-int host_statfs(const char *path, struct nuttx_statfs_s *buf)
+int host_statvfs(const char *path, struct nuttx_statvfs_s *buf)
{
int ret;
struct statvfs hostbuf;
- /* Call the host's statfs routine */
+ /* Call the host's statvfs routine */
ret = statvfs(path, &hostbuf);
if (ret < 0)
{
ret = -errno;
}
- /* Map the struct statfs value */
+ /* Map the struct statvfs value */
- buf->f_type = 0; /* hostfs overwrites f_type anyway */
- buf->f_namelen = hostbuf.f_namemax;
+ buf->f_fsid = 0; /* hostfs overwrites f_fsid anyway */
+ buf->f_flag = hostbuf.f_flag;
Review comment:
f_flags needs host->nuttx conversion. maybe it's safer to use 0 for now.
##########
File path: include/sys/statvfs.h
##########
@@ -49,7 +49,14 @@
* Pre-processor Definitions
****************************************************************************/
-/* struct statfs file system types. */
+/* Definitions for the flag in `f_flag'. These definitions should be
+ * kept in sync with the definitions in <sys/mount.h>.
+ */
+
+#define ST_RDONLY 0x0001 /* Mount read-only. */
+#define ST_NOSUID 0x0002 /* Ignore suid and sgid bits. */
Review comment:
we don't have NOSUID things.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r449744290
##########
File path: arch/sim/src/sim/up_hostfs.c
##########
@@ -432,32 +432,35 @@ int host_closedir(void *dirp)
}
/****************************************************************************
- * Name: host_statfs
+ * Name: host_statvfs
****************************************************************************/
-int host_statfs(const char *path, struct nuttx_statfs_s *buf)
+int host_statvfs(const char *path, struct nuttx_statvfs_s *buf)
{
int ret;
struct statvfs hostbuf;
- /* Call the host's statfs routine */
+ /* Call the host's statvfs routine */
ret = statvfs(path, &hostbuf);
if (ret < 0)
{
ret = -errno;
}
- /* Map the struct statfs value */
+ /* Map the struct statvfs value */
- buf->f_type = 0; /* hostfs overwrites f_type anyway */
- buf->f_namelen = hostbuf.f_namemax;
+ buf->f_fsid = 0; /* hostfs overwrites f_fsid anyway */
+ buf->f_flag = hostbuf.f_flag;
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 merged pull request #1355: fs/vfs: Implement statvfs and fstatvfs
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656267247
@yamt do you have more concern about the change? If no, please merge this patch and apache/incubator-nuttx-apps#325. The check failure has to be ignored since the change in nuttx and apps need sync to pass the compile.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656624392
@xiaoxiang781216 i think f_fsid is meant to be an ID for each individual file system, not file system type. how do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1355: vfs: Make the filesystem query API confirm POSIX standard
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r449744533
##########
File path: include/sys/statvfs.h
##########
@@ -49,7 +49,14 @@
* Pre-processor Definitions
****************************************************************************/
-/* struct statfs file system types. */
+/* Definitions for the flag in `f_flag'. These definitions should be
+ * kept in sync with the definitions in <sys/mount.h>.
+ */
+
+#define ST_RDONLY 0x0001 /* Mount read-only. */
+#define ST_NOSUID 0x0002 /* Ignore suid and sgid bits. */
Review comment:
Yes, but it isn't harmful to keep it here since we may imlement this feature someday.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1355: fs/vfs: Implement statvfs and fstatvfs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r453411778
##########
File path: include/sys/statvfs.h
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * include/sys/statvfs.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_STATVFS_H
+#define __INCLUDE_SYS_STATVFS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/statfs.h>
Review comment:
Done, @yamt.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1355: fs/vfs: Implement statvfs and fstatvfs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r453407442
##########
File path: include/sys/statvfs.h
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * include/sys/statvfs.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_STATVFS_H
+#define __INCLUDE_SYS_STATVFS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/statfs.h>
Review comment:
Not necessary, I changed to sys/types.h
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1355: fs/vfs: Implement statvfs and fstatvfs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#discussion_r453405671
##########
File path: include/sys/statvfs.h
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * include/sys/statvfs.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_STATVFS_H
+#define __INCLUDE_SYS_STATVFS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/statfs.h>
Review comment:
is this include necessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1355: fs/vfs: Implement statvfs and fstatvfs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1355:
URL: https://github.com/apache/incubator-nuttx/pull/1355#issuecomment-656729787
@ymat, the new patch implement statvfs/fstatvfs on top of statfs/fstatfs. Please review again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org