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 2021/08/09 08:22:34 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #4300: fs: Add utimens and lutimens
xiaoxiang781216 opened a new pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300
## Summary
https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
https://www.daemon-systems.org/man/utimens.2.html
## Impact
New API, follow up the change from https://github.com/apache/incubator-nuttx/pull/4153
## 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] xiaoxiang781216 commented on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895264676
@gustavonihei I append a new patch to this PR, you can see from it that:
1. The sim implementation of hostfs call utimensat:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
2. The rpmsg implmentation of ostfs call utimens:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779
When utimensat is ready, rpmsg's version will change to call utimensat too.
--
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] gustavonihei commented on a change in pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#discussion_r685261970
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -217,7 +226,7 @@ int host_mkdir(const char *pathname, mode_t mode);
int host_rmdir(const char *pathname);
int host_rename(const char *oldpath, const char *newpath);
int host_stat(const char *path, struct stat *buf);
-
+int host_stat(const char *path, const struct stat *buf, int flags);
Review comment:
```suggestion
int host_chstat(const char *path, const struct stat *buf, int flags);
```
I think there is a mistake here on the function declaration.
--
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] xiaoxiang781216 edited a comment on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895264676
@gustavonihei I append a new patch to this PR, you can see from it that:
1. The sim implementation of hostfs call utimensat:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
2. The rpmsg implmentation of hostfs call utimens:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779
When utimensat is ready, rpmsg's version will change to call utimensat too.
--
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] gustavonihei commented on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895216033
I am a bit confused here. I am not familiar with these APIs.
According to the latest release of the OpenGroup standard, neither `utimens` nor `lutimens` are standardized functions.
I saw that `utimes` has been moved the `XSI` and been somewhat deprecated.
But, the standard defines `utimensat` as an equivalent function for `utimes`.
Isn't `utimensat` the implementation you intended to add instead of `utimens`?
--
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] xiaoxiang781216 commented on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895255217
> I am a bit confused here. I am not familiar with these APIs.
> According to the latest release of the OpenGroup standard, neither `utimens` nor `lutimens` are standardized functions.
> I saw that `utimes` has been moved the `XSI` and been somewhat deprecated.
> But, the standard defines `utimensat` as an equivalent function for `utimes`.
> Isn't `utimensat` the implementation you intended to add instead of `utimens`?
Yes, utimensat is the final goal: @Donny9 is preparing a patch series to support utimensat(and other similar function e.g. openat, unlinkat...). This PR could be used before that patch finish. Once `utimensat` is ready, all these time related functions will simply forward to `utimensat`.
--
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] gustavonihei commented on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895406957
> @gustavonihei I append a new patch to this PR, you can see from it that:
>
> 1. The sim implementation of hostfs call utimensat:
> [d7631be#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646](https://github.com/apache/incubator-nuttx/commit/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646)
> 2. The rpmsg implmentation of hostfs call utimens:
> [d7631be#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779](https://github.com/apache/incubator-nuttx/commit/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779)
>
> Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.
Ok, I got your point, I think it makes sense.
So `utimens` and `lutimens` usage will later be restricted as just internal helper functions.
LGTM, but the CI is still failing on some compiler errors.
--
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] xiaoxiang781216 edited a comment on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895264676
@gustavonihei I append a new patch to this PR, you can see from it that:
1. The sim implementation of hostfs call utimensat:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
2. The rpmsg implmentation of hostfs call utimens:
https://github.com/apache/incubator-nuttx/pull/4300/commits/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779
Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.
--
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] gustavonihei merged pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300
--
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] xiaoxiang781216 commented on a change in pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#discussion_r685272997
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -217,7 +226,7 @@ int host_mkdir(const char *pathname, mode_t mode);
int host_rmdir(const char *pathname);
int host_rename(const char *oldpath, const char *newpath);
int host_stat(const char *path, struct stat *buf);
-
+int host_stat(const char *path, const struct stat *buf, int flags);
Review comment:
Yes, 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.
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] gustavonihei commented on a change in pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#discussion_r685263497
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -132,8 +136,8 @@ struct nuttx_timespec
struct nuttx_dirent_s
{
- uint8_t d_type; /* type of file */
- char d_name[NUTTX_NAME_MAX + 1]; /* filename */
+ uint8_t d_type; /* type of file */
+ char d_name[CONFIG_NAME_MAX + 1]; /* filename */
Review comment:
```suggestion
char d_name[NAME_MAX + 1]; /* filename */
```
It is better to link to the definitions from `limits.h`, isn't it?
--
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] xiaoxiang781216 commented on pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#issuecomment-895418503
> > @gustavonihei I append a new patch to this PR, you can see from it that:
> >
> > 1. The sim implementation of hostfs call utimensat:
> > [d7631be#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646](https://github.com/apache/incubator-nuttx/commit/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646)
> > 2. The rpmsg implmentation of hostfs call utimens:
> > [d7631be#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779](https://github.com/apache/incubator-nuttx/commit/d7631be5e1e00c3d5fea0b085f02cac596fc05ec#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779)
> >
> > Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.
>
> Ok, I got your point, I think it makes sense.
> So `utimens` and `lutimens` usage will later be restricted as just internal helper functions.
Yes, it will be replace with utimensat.
> LGTM, but the CI is still failing on some compiler errors.
It should OK, now.
--
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] xiaoxiang781216 commented on a change in pull request #4300: fs: Add utimens and lutimens
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4300:
URL: https://github.com/apache/incubator-nuttx/pull/4300#discussion_r685271658
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -132,8 +136,8 @@ struct nuttx_timespec
struct nuttx_dirent_s
{
- uint8_t d_type; /* type of file */
- char d_name[NUTTX_NAME_MAX + 1]; /* filename */
+ uint8_t d_type; /* type of file */
+ char d_name[CONFIG_NAME_MAX + 1]; /* filename */
Review comment:
We can't change to NAME_MAX here, because hostfs.h is also included by the code compiled in PC environment.
--
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