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 2023/01/16 11:27:03 UTC
[GitHub] [nuttx] XuNeo opened a new pull request, #8138: fs/mmap: fix mmap returned address
XuNeo opened a new pull request, #8138:
URL: https://github.com/apache/nuttx/pull/8138
## Summary
- 1. rammap should return memory start address
- 2. update output `mapped` pointer in file_mmap_ before returning
## Impact
No.
## Testing
mmap used in gettext now works 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071200247
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
the goto errout_with_region on lines 215 and 245 are actually wrong as well; the pointer to rdbuffer is no longer pointing to the allocated area, but is still freed there
--
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] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071201320
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
In the "free" below on lines 253, 257 we should use entry->vaddr, since that stores the original allocated ptr
--
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] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071201320
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
The "rdbuffer" pointer is no longer pointing to the original allocated pointer, when gotoed from lines 215, 245. In the "free" below on lines 253, 257 we should use entry->vaddr, since that stores the original allocated ptr
--
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] [nuttx] XuNeo commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
XuNeo commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071346223
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
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] [nuttx] xiaoxiang781216 commented on pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#issuecomment-1384780481
Let's ignore the unrelated ci broken which is fixed by https://github.com/apache/nuttx/pull/8155.
--
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] [nuttx] jlaitine commented on pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#issuecomment-1384058154
Clarified my original comment, it was too quickly written to be understandable....
--
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] [nuttx] xiaoxiang781216 commented on pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#issuecomment-1383920967
@jlaitine could you review the change? we found some regression from https://github.com/apache/nuttx/pull/8026
--
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] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071200247
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
the next ret check with goto errout_with_region is actually wrong as well; the pointer to rdbuffer is no longer pointing to the allocated area, but is still freed there
--
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] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071200247
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
the goto errout_with_region on lines 215 and 245 are actually wrong as well; the pointer to rdbuffer is no longer pointing to the allocated area, but is still freed there
--
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] [nuttx] jlaitine commented on a diff in pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#discussion_r1071201320
##########
fs/mmap/fs_rammap.c:
##########
@@ -235,7 +237,6 @@ int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry,
/* Add the buffer to the list of regions */
- entry->vaddr = rdbuffer;
entry->priv.i = kernel;
entry->munmap = unmap_rammap;
Review Comment:
In the "free" below we should use entry->vaddr, since that stores the original allocated ptr
--
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] [nuttx] xiaoxiang781216 merged pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #8138:
URL: https://github.com/apache/nuttx/pull/8138
--
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] [nuttx] XuNeo commented on pull request #8138: fs/mmap: fix mmap returned address
Posted by GitBox <gi...@apache.org>.
XuNeo commented on PR #8138:
URL: https://github.com/apache/nuttx/pull/8138#issuecomment-1384065337
Let me update the commit later.
For `int rammap(FAR struct file *filep, FAR struct mm_map_entry_s *entry, bool kernel)`, am I correct to add comment to say that `entry->offset` and `entry->length` must be initialized ?
--
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