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/03/26 17:56:43 UTC
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5854: fs/romfs: fix bug about test case:examples/romfs
pkarashchenko commented on a change in pull request #5854:
URL: https://github.com/apache/incubator-nuttx/pull/5854#discussion_r835791057
##########
File path: fs/romfs/fs_romfsutil.c
##########
@@ -436,51 +429,44 @@ static int romfs_cachenode(FAR struct romfs_mountpt_s *rm,
return ret;
}
- /* Now we are pointing to the real entry of interest. Is it a
- * directory? Or a file?
- */
+ ret = romfs_parsefilename(rm, offset, childname);
+ if (ret < 0)
+ {
+ return ret;
+ }
- if (IS_DIRECTORY(next) || IS_FILE(next))
+ if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
{
- ret = romfs_parsefilename(rm, offset, childname);
- if (ret < 0)
+ if (child == NULL || nodeinfo->rn_count == num - 1)
{
- return ret;
- }
+ FAR void *tmp;
- if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
- {
- if (child == NULL || nodeinfo->rn_count == num - 1)
+ tmp = kmm_realloc(nodeinfo->rn_child,
+ (num + NODEINFO_NINCR) *
+ sizeof(FAR struct romfs_nodeinfo_s *));
+ if (tmp == NULL)
{
- FAR void *tmp;
-
- tmp = kmm_realloc(nodeinfo->rn_child,
- (num + NODEINFO_NINCR) *
- sizeof(FAR struct romfs_nodeinfo_s *));
- if (tmp == NULL)
- {
- return -ENOMEM;
- }
-
- nodeinfo->rn_child = tmp;
- memset(nodeinfo->rn_child + num, 0, NODEINFO_NINCR *
- sizeof(FAR struct romfs_nodeinfo_s *));
- num += NODEINFO_NINCR;
+ return -ENOMEM;
}
- child = &nodeinfo->rn_child[nodeinfo->rn_count++];
- if (IS_DIRECTORY(next))
- {
- linkoffset = info;
- }
+ nodeinfo->rn_child = tmp;
+ memset(nodeinfo->rn_child + num, 0, NODEINFO_NINCR *
+ sizeof(FAR struct romfs_nodeinfo_s *));
Review comment:
```suggestion
sizeof(struct romfs_nodeinfo_s *));
```
##########
File path: fs/romfs/fs_romfsutil.c
##########
@@ -436,51 +429,44 @@ static int romfs_cachenode(FAR struct romfs_mountpt_s *rm,
return ret;
}
- /* Now we are pointing to the real entry of interest. Is it a
- * directory? Or a file?
- */
+ ret = romfs_parsefilename(rm, offset, childname);
+ if (ret < 0)
+ {
+ return ret;
+ }
- if (IS_DIRECTORY(next) || IS_FILE(next))
+ if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
{
- ret = romfs_parsefilename(rm, offset, childname);
- if (ret < 0)
+ if (child == NULL || nodeinfo->rn_count == num - 1)
{
- return ret;
- }
+ FAR void *tmp;
- if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
- {
- if (child == NULL || nodeinfo->rn_count == num - 1)
+ tmp = kmm_realloc(nodeinfo->rn_child,
+ (num + NODEINFO_NINCR) *
+ sizeof(FAR struct romfs_nodeinfo_s *));
+ if (tmp == NULL)
{
- FAR void *tmp;
-
- tmp = kmm_realloc(nodeinfo->rn_child,
- (num + NODEINFO_NINCR) *
- sizeof(FAR struct romfs_nodeinfo_s *));
- if (tmp == NULL)
- {
- return -ENOMEM;
- }
-
- nodeinfo->rn_child = tmp;
- memset(nodeinfo->rn_child + num, 0, NODEINFO_NINCR *
- sizeof(FAR struct romfs_nodeinfo_s *));
- num += NODEINFO_NINCR;
+ return -ENOMEM;
}
- child = &nodeinfo->rn_child[nodeinfo->rn_count++];
- if (IS_DIRECTORY(next))
- {
- linkoffset = info;
- }
+ nodeinfo->rn_child = tmp;
Review comment:
```suggestion
nodeinfo->rn_child = (FAR struct romfs_nodeinfo_s **)tmp;
```
##########
File path: fs/romfs/fs_romfsutil.c
##########
@@ -108,44 +108,37 @@ static inline int romfs_checkentry(FAR struct romfs_mountpt_s *rm,
return ret;
}
- /* Now we are pointing to the real entry of interest. Is it a
- * directory? Or a file?
+ /* Get the name of the directory entry. */
+
+ ret = romfs_parsefilename(rm, offset, name);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ /* Then check if this the name segment we are looking for. The
+ * string comparison is awkward because there is no terminator
+ * on entryname (there is a terminator on name, however)
*/
- if (IS_DIRECTORY(next) || IS_FILE(next))
+ if (memcmp(entryname, name, entrylen) == 0 &&
+ strlen(name) == entrylen)
Review comment:
```suggestion
if (strlen(name) == entrylen &&
memcmp(entryname, name, entrylen) == 0)
```
##########
File path: fs/romfs/fs_romfsutil.c
##########
@@ -436,51 +429,44 @@ static int romfs_cachenode(FAR struct romfs_mountpt_s *rm,
return ret;
}
- /* Now we are pointing to the real entry of interest. Is it a
- * directory? Or a file?
- */
+ ret = romfs_parsefilename(rm, offset, childname);
+ if (ret < 0)
+ {
+ return ret;
+ }
- if (IS_DIRECTORY(next) || IS_FILE(next))
+ if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
{
- ret = romfs_parsefilename(rm, offset, childname);
- if (ret < 0)
+ if (child == NULL || nodeinfo->rn_count == num - 1)
{
- return ret;
- }
+ FAR void *tmp;
- if (strcmp(childname, ".") != 0 && strcmp(childname, "..") != 0)
- {
- if (child == NULL || nodeinfo->rn_count == num - 1)
+ tmp = kmm_realloc(nodeinfo->rn_child,
+ (num + NODEINFO_NINCR) *
+ sizeof(FAR struct romfs_nodeinfo_s *));
Review comment:
```suggestion
sizeof(struct romfs_nodeinfo_s *));
```
actually we can replace it even with `void *`, but change to be `sizeof(*nodeinfo->rn_child)` seems to be even more reasonable.
--
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