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/10/15 07:11:04 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #7317: Refine the implementation of rename
xiaoxiang781216 opened a new pull request, #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317
## Summary
- fs/vfs: Free subdir before allocate new one in pseudorename
- fs/vfs: Remove the redundancy file name comparison in mountptrename
## Impact
Minor
## 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 a diff in pull request #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317#discussion_r996432560
##########
fs/vfs/fs_rename.c:
##########
@@ -90,7 +90,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
{
inode_release(newinode);
ret = OK;
- goto errout; /* Bad naming, this is not an error case. */
+ goto errout; /* Same name, this is not an error case. */
Review Comment:
Could be
--
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 diff in pull request #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317#discussion_r996337998
##########
fs/vfs/fs_rename.c:
##########
@@ -111,26 +111,21 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
if (newinode->u.i_ops == NULL || newinode->i_child != NULL)
{
FAR char *subdirname;
- FAR char *tmp;
inode_release(newinode);
+ if (subdir != NULL)
+ {
+ kmm_free(subdir);
+ subdir = NULL;
+ }
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.
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 #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317#discussion_r996418794
##########
fs/vfs/fs_rename.c:
##########
@@ -90,7 +90,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
{
inode_release(newinode);
ret = OK;
- goto errout; /* Bad naming, this is not an error case. */
+ goto errout; /* Same name, this is not an error case. */
Review Comment:
Yeah. Probably will need to update `cmd_mv` in NSH
--
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] acassis merged pull request #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
acassis merged PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317
--
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] acassis commented on a diff in pull request #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317#discussion_r996324092
##########
fs/vfs/fs_rename.c:
##########
@@ -90,7 +90,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
{
inode_release(newinode);
ret = OK;
- goto errout; /* Bad naming, this is not an error case. */
+ goto errout; /* Same name, this is not an error case. */
Review Comment:
Should it be considered an error? At least on Linux when trying to move a directory to itself we see an error reported:
```
$ mv a a
mv: cannot move 'a' to a subdirectory of itself, 'a/a'
```
##########
fs/vfs/fs_rename.c:
##########
@@ -111,26 +111,21 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
if (newinode->u.i_ops == NULL || newinode->i_child != NULL)
{
FAR char *subdirname;
- FAR char *tmp;
inode_release(newinode);
+ if (subdir != NULL)
+ {
+ kmm_free(subdir);
+ subdir = NULL;
+ }
Review Comment:
Please include a comment above, your modification simplified the silly test below, but for someone reading this code these things are difficult to figure-out initially, this is a loop function.
--
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 diff in pull request #7317: Refine the implementation of rename
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7317:
URL: https://github.com/apache/incubator-nuttx/pull/7317#discussion_r996337161
##########
fs/vfs/fs_rename.c:
##########
@@ -90,7 +90,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
{
inode_release(newinode);
ret = OK;
- goto errout; /* Bad naming, this is not an error case. */
+ goto errout; /* Same name, this is not an error case. */
Review Comment:
@acassis the message is reported by mv command, not function Here is my test on WSL2:
```
$ cat test.c
#include <stdio.h>
int main(int argc, char *argv[])
{
return rename(argv[1], argv[2]);
}
$ gcc test.c
$ ./a.out a.out a.out
$ echo $?
0
```
--
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