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/08/06 10:58:24 UTC
[GitHub] [incubator-nuttx] davids5 opened a new pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
davids5 opened a new pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522
## Summary
PR #1372 vfs/stat: Make the flag defintion more confirm POSIX standard. Silently broke the cromfs. This was due to the necessary replication of the S_xxxx flags of the NuttX sys/stat.h. The unfortunate outcome was most likely caused by fact, that a grep of S_xxxx did not match the tools/gencromfs.c
To insure this sort of thing does not happen again, comments have been added the will match search.
## Impact
Critical all system using cromfs are broken on master
It is clear our test coverage is incomplete. Maybe the cromfs test app should be added to the sim target and run?
## Testing
Before PR:
```
[boot] Fault Log is Armed
nsh: init: fopen failed: Not a directory
```
After PR
```
[boot] Fault Log is Armed
set AUTOCNF no
set AUX_MODE pwm
set DATAMAN_OPT ""
set FAILSAFE none
set FAILSAFE_AUX none
set FCONFIG /fs/microsd/etc/config.txt
set FEXTRAS /fs/microsd/etc/extras.txt
set FMU_MODE pwm
[hardfault_log] Fault Log is Armed
set FRC /fs/microsd/etc/rc.txt
```
----------------------------------------------------------------
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] davids5 commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670156116
@xiaoxiang781216 @patacongo - I ran into a secondary problem. Even thougj I could execute the script. The /etc directory was not listed with ls -R /.
```
/etc:
nsh: ls: no such directory: /etc
```
The change to the sys/stat.h file made discrete code points out of all the types. Before there were code points and attributes.
The former code was logically combinable and testable: If the link type was added to dir type resulted in a dirlink. The operation IS_DIR(m) was true with m=DIR or m = a Linked Dir. Now the combination is not testable. The result is as shown below:
| was | is |
-- | -- | -- | --
S_ISLNK | 0x8000 | 0xa000 | A link
S_ISDIR | 0x1000 | 0x4000 | A Dir
Both | 0x9000 | 0xE000 | A dir/link
s_IFTGT | 0x7800 | | Old Isolation mask
S_IFMT | | 0xF000 | New Isolation mask
Both & s_IFTGT | 0x1000 | | Is A directory
Both & S_IFMT | | 0XE000 | **Is not a testable type**
----------------------------------------------------------------
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 merged pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522
----------------------------------------------------------------
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] davids5 commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-669924103
@xiaoxiang781216 what it the current rule on merging? Can I or what are you waiting for?
----------------------------------------------------------------
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 #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670167504
Link flag can't or with dir flag: link flag tell you this is a link, If you want to know whether it is a directory, file, socket, block or char, you have to follow the link and ask the link target type.
You can see Linux definition almost same with NuttX:
https://github.com/bminor/glibc/blob/5f72f9800b250410cad3abfeeb09469ef12b2438/sysdeps/unix/sysv/linux/generic/bits/stat.h#L146
BTW, is etc/ a mount point?
----------------------------------------------------------------
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] patacongo commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670223690
This is probably affected too: https://bitbucket.org/nuttx/tools/src/master/nxfuse/
----------------------------------------------------------------
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 edited a comment on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670167504
Link flag can't or with dir flag: link flag tell you this is a link, If you want to know whether it is a directory, file, socket, block or char, you have to follow the link and ask the link target type:
1.stat always follow the link and report the target type so never return link flag
2.lstat never follow the link and report the type is link
You can see Linux definition almost same with NuttX:
https://github.com/bminor/glibc/blob/5f72f9800b250410cad3abfeeb09469ef12b2438/sysdeps/unix/sysv/linux/generic/bits/stat.h#L146
BTW, is etc/ a mount point?
----------------------------------------------------------------
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 #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670461967
> This is probably affected too: https://bitbucket.org/nuttx/tools/src/master/nxfuse/
>
> Can someone confirm that (by inspection at least)? If so we should fix that too or open an Issue.
I will check the code today.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670223690
This is probably affected too: https://bitbucket.org/nuttx/tools/src/master/nxfuse/
Can someone confirm that (by inspection at least)? If so we should fix that too or open an Issue.
----------------------------------------------------------------
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] davids5 commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-669860956
Please note:
Remaining violation seem to be an issue with the nxstyle parsing `outptr[- lit - 1] = lit - 1` It look fin to me, any suggestion on a work around?
----------------------------------------------------------------
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] davids5 commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670190335
@xiaoxiang781216 see https://github.com/apache/incubator-nuttx/pull/1524
----------------------------------------------------------------
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] davids5 commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670183497
>Link flag can't or with dir flag: link flag tell you this is a link, If you want to know whether it is a directory, file, socket, block or char, you have to follow the link and ask the link target type:
This [line](https://github.com/nuttx-to-asf/incubator-nuttx/blob/c75bbb271583f091402b32307fa047e2a3ea705a/tools/gencromfs.c#L113) was creating a DIRLink and S_ISDIR worked on it. It is used for the "." and ".." So I think if I just set it to a link and test for both a DIR and Link in cromfs_opendir that should work.
Is there more I should do?
> BTW, is etc/ a mount point?
Yes.
----------------------------------------------------------------
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] patacongo commented on pull request #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-669926278
> what it the current rule on merging? Can I or what are you waiting for?
We NEVER merge our own PRs.
----------------------------------------------------------------
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 #1522: Fix Breakage from 91ed14c in PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1522:
URL: https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670964000
nxfuse don't check link flag because smartfs and nxffs don't support the soft link.
----------------------------------------------------------------
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