-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
some tiny improvements and way more comments #162
base: develop
Are you sure you want to change the base?
some tiny improvements and way more comments #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, these changes look good - there are a couple things I'd want changed, but probably worth a review from @lurch too?
* @param vaddr Virtual address. | ||
* @param size Size of the area to check. | ||
* @param uninitialized Flag indicating if the area is uninitialized. | ||
* @param ar Reference to an address_range that will be set on success. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Reference to the valid_range that the provided (addr + size) fits inside" ?
* | ||
* @param entries Vector of ELF-32 program header entries. | ||
* @param valid_ranges Valid address ranges. | ||
* @param pages Map of pages with their fragments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Reference to the page fragments map", to make it more obvious that this is "the page fragments map" to which the @brief
is referring?
* @param package_addr Optional packaging address. | ||
* @param abs_block_loc Optional absolute block location. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same questions about "Optional"
Co-authored-by: will-v-pi <[email protected]>
Signed-off-by: Simon Waldherr <[email protected]>
Signed-off-by: Simon Waldherr <[email protected]>
So, what's the status? Would you like any other changes? |
There are a couple of unresolved comments from @lurch still - but once those are resolved this can be merged |
pull request changed to "develop"-branch as requested by @will-v-pi (#160)