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 21:02:30 UTC
[GitHub] [incubator-nuttx] davids5 opened a new pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
davids5 opened a new pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524
## Summary
Follow on to https://github.com/apache/incubator-nuttx/pull/1522
Prior to PR 1372 vfs/stat: Make the flag defintion more confirm POSIX standard. #1522
The link flag and the type we combined for ".", ".." as DIRLINK after #1522 the types are not OR-able.
## Impact
cromfs directory listing are failing on master.
## Testing
` ls -R /` Now works.
```
/etc:
.
init.d/
extras/
mixers/
/etc/init.d:
.
..
rc.uuv_apps
```
----------------------------------------------------------------
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 a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466951394
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
@xiaoxiang781216
Results of your suggested change are below. It breaks it again.
Please merge this PR. It is correct in the context of the cromfs and gencromfs.c
```
nsh> ls -R /
/:
dev/
etc/
fs/
obj/
proc/
/dev:
baro0
console
console_buf
led0
mag0
mag1
mmcsd0
mtdblock0
mtdblock1
null
tone_alarm0
ttyACM0
ttyS0
ttyS1
ttyS2
ttyS3
ttyS4
ttyS5
ttyS6
uavcan/
/dev/uavcan:
airspeed
baro
battery
differential_pressure
esc
flow
gnss
mag
/etc:
nsh: ls: no such directory: /etc
nsh>
```
I think you need to understand the the ripple effects the change you made to the total set of fs's. I have tried to explain it it in the other PR and it was clear for your response, we are not communicating clearly.
I understand that following the link is the {uni|pos}ix way, but the PR 1372 `vfs/stat: Make the flag defintion more confirm POSIX standard.` broke the nuttx way on **_ALL_** fs's that depended on combinatorial operations. [see Greg's comment](https://github.com/apache/incubator-nuttx/pull/1522#issuecomment-670223690)
Please test master on the sim with a cromfs and all the others fs that may have depended on the following operations
`(m) & TYPE_MASK == TYPE` and `(m) & LINK_MASK == LINK` were m is (TYPE|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
[GitHub] [incubator-nuttx] xiaoxiang781216 closed pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #1524:
URL: 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] xiaoxiang781216 commented on a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466790485
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
for link we need look at the target to decide whether is a directory, If we don't want to support the link now, it's better to:
if (S_ISLNK(node->cn_mode))
{
/* Not implement yet */
return -ENOSYS;
}
else if (!S_ISDIR(node->cn_mode))
{
return -ENOTDIR;
}
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466965550
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
BTW, if you are using CROMFS, please add a config to enable the automation build. If you don't take care, nobody can ensure it will always build and work as expect. As I suggest you before to enable the out of board test.
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671257599
> @davids5 please try this patch: #1524
1524 is this PR. What is it you wanted me to test?
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466974169
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
After comparing gencromfs.c and https://github.com/chexum/genromfs/blob/master/genromfs.c, it's better to change the link to normal directory:
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1019
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1020
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1375
because ROMFS is done like this:
https://github.com/chexum/genromfs/blob/master/genromfs.c#L810
https://github.com/chexum/genromfs/blob/master/genromfs.c#L823
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671051517
@xiaoxiang781216 You broke the FS. Whether or not the flag this implementation chooses to use is up to the implementation. This PR restored what you broke. You should fix what you broke or bring this in until you have a full tested implementation of all the things effected by the FS change.
----------------------------------------------------------------
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 edited a comment on pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671257599
> @davids5 please try this patch: #1524
@xiaoxiang781216 1524 is this PR. What is it you wanted me to test?
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466974169
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
After comparing gencromfs.c and https://github.com/chexum/genromfs/blob/master/genromfs.c, it's better to change the link to normal directory:
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1019
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1020
https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1375
because ROMFS is done by this way:
https://github.com/chexum/genromfs/blob/master/genromfs.c#L810
https://github.com/chexum/genromfs/blob/master/genromfs.c#L823
----------------------------------------------------------------
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 a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r467027343
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
> As I suggest you before to enable the out of board test.
I hope you meant on the sim.
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671052990
@davids5 the fix is wrong, I give up the vote. @patacongo you make the final decision whether we merge @davids5 's workaround or fix it correctly.
----------------------------------------------------------------
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 a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466980706
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
@xiaoxiang781216 Why would @patacongo done the extra work, if it was not needed? The operation on the cromfs look very different to me (one is recursive, the other is not). If you do do a separate PR, I would be happy to test 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671320223
Replaced by https://github.com/apache/incubator-nuttx/pull/1550
@patacongo thank you for pointing out what @xiaoxiang781216 has been trying to tell me.
----------------------------------------------------------------
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 a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r467026716
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
> BTW, if you are using CROMFS, please add a config to enable the automation build. If you don't take care, nobody can ensure it will always build and work as expect. As I suggest you before to enable the out of board test.
Done but I need some info....
https://github.com/apache/incubator-nuttx/pull/1532
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466994644
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
I am fine with the current change, @patacongo please merge it if you don't have more concern.
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466962422
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
I already check the whole base to ensure there isn't | with S_IFLNK before send the PR. The problem is that genromfs use the different name to identify the link which is hard to search.
----------------------------------------------------------------
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 a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466980706
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
@xiaoxiang781216 Why would @patacongo done the extra work, if it was not needed? The dir and hardlink operation on the cromfs look very different to me (one is recursive, the other is not). If you do do a separate PR, I would be happy to test 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466965871
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
@xiaoxiang781216 - Yes I know, but the important point is **there may be more of them**!
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-670457405
@xiaoxiang781216 Are you going to merge this PR or did you want to fix it another way?
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671060257
> @xiaoxiang781216 The internal contract between the data, and encoding it correct in the the generator and the fs driver. What are you calling wrong? Please describe what is "wrong".
Because gencromfs mix the usage of hard link and soft link when generating '.' and '..' entries:
1.Hard link directly point to the real data block without any special flag. VFS don't need handle the hard link specially.
2.Soft link's data contain a symbol link to the target and then a special flag(LINK) is required to tell VFS handle it specially.
I have mentioned the correct fix before:
> After comparing gencromfs.c and https://github.com/chexum/genromfs/blob/master/genromfs.c, it's better to change the link to normal directory:
> https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1019
> https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1020
> https://github.com/apache/incubator-nuttx/blob/master/tools/gencromfs.c#L1375
> because ROMFS is done by this way:
> https://github.com/chexum/genromfs/blob/master/genromfs.c#L810
> https://github.com/chexum/genromfs/blob/master/genromfs.c#L823
We need the real(no LINK flag) hardlink here. This article describe the difference:
https://linuxhint.com/soft_link_vs_hard_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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671007891
The fix is wrong. '.' and '..' is the hard link which shouldn't set LINK flag.
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466790485
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
For link we need look at the target to decide whether is a directory, If we don't want to support the link now, it's better to:
```
if (S_ISLNK(node->cn_mode))
{
/* Not implement yet */
return -ENOSYS;
}
else if (!S_ISDIR(node->cn_mode))
{
return -ENOTDIR;
}
```
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671052990
@davids5 the fix is wrong, I give up the vote. @patacongo you make the final decision whether we merge @davids5 's patch or fix it correctly.
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671053538
@xiaoxiang781216 The internal contract between the data, and encoding it correct in the the generator and the fs driver. What are you calling wrong? Please describe what is "wrong".
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-670954634
@davids5 please try this patch: 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] xiaoxiang781216 commented on a change in pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#discussion_r466790485
##########
File path: fs/cromfs/fs_cromfs.c
##########
@@ -988,7 +988,7 @@ static int cromfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
/* Verify that the node is a directory */
- if (!S_ISDIR(node->cn_mode))
+ if (!S_ISDIR(node->cn_mode) && !S_ISLNK(node->cn_mode))
Review comment:
For link we need look at the target to decide whether is a directory, If we don't want to support the link now, it's better to:
if (S_ISLNK(node->cn_mode))
{
/* Not implement yet */
return -ENOSYS;
}
else if (!S_ISDIR(node->cn_mode))
{
return -ENOTDIR;
}
----------------------------------------------------------------
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 #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1524:
URL: https://github.com/apache/incubator-nuttx/pull/1524#issuecomment-671007891
The fix is wrong. '.' and '..' is the hard link which shouldn't set LINK flag. I opened an issue track this: https://github.com/apache/incubator-nuttx/issues/1540
----------------------------------------------------------------
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 closed pull request #1524: cromfs:gencromfs fs_cromfs Fix special directory handling
Posted by GitBox <gi...@apache.org>.
davids5 closed pull request #1524:
URL: 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