-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
arch: coding guidelines: use bool param when data nature is boolean #72675
arch: coding guidelines: use bool param when data nature is boolean #72675
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.
I'm happy with the change in the Arm part.
However, the doc for the API should most likely be updated to be consistent with the new data type.
converted buffer validation param "write" to boolean because this param is used as a boolean. Signed-off-by: Hess Nathan <[email protected]>
aa08664
to
c521d23
Compare
You're right, updated the doc accordingly |
@@ -207,7 +207,7 @@ int arc_core_mpu_get_max_domain_partition_regions(void) | |||
/** | |||
* @brief validate the given buffer is user accessible or not | |||
*/ | |||
int arc_core_mpu_buffer_validate(const void *addr, size_t size, int write) | |||
int arc_core_mpu_buffer_validate(const void *addr, size_t size, bool write) |
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.
This function calls _is_user_accessible_region
, shouldn't that be updated as well?
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.
We need to update _is_user_accessible_region
(which is called from arc_core_mpu_buffer_validate
) as well.
Currently it's defined as
static inline bool _is_user_accessible_region(uint32_t r_index, int write)
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.
Hm... this is an ancient function whose app-side responsibility has largely migrated to the mem_domain APIs. We don't actually call it anywhere in the tree except tests.
Might be worth considering removing it instead of needlessly churning its API...
@andyross How would I deal with that? |
@@ -207,7 +207,7 @@ int arc_core_mpu_get_max_domain_partition_regions(void) | |||
/** | |||
* @brief validate the given buffer is user accessible or not | |||
*/ | |||
int arc_core_mpu_buffer_validate(const void *addr, size_t size, int write) | |||
int arc_core_mpu_buffer_validate(const void *addr, size_t size, bool write) |
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.
We need to update _is_user_accessible_region
(which is called from arc_core_mpu_buffer_validate
) as well.
Currently it's defined as
static inline bool _is_user_accessible_region(uint32_t r_index, int write)
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
converted arch buffer validation param "write" to boolean because this param is used as a boolean in the method.
This PR is part of the enhancement issue #48002 which port the coding guideline fixes done by BUGSENG on the https://github.com/zephyrproject-rtos/zephyr/tree/v2.7-auditable-branch back to main
The commit in this PR is a subset of the original auditable-branch commit:
d03fa8d