Skip to content
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

refine code comments: clarify misleading names and comments for thread_rpc_alloc_*_payload APIs #7096

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

santongding
Copy link

The thread_rpc_alloc_*_payload APIs are consistent with the thread_shm_type enumerations, as seen in the alloc_shm function. However, the name and comments of thread_rpc_alloc_payload might be misleading, suggesting it is a universal API. To maintain backward compatibility, only the code comments have been updated for clarity.

…ad_rpc_alloc_*_payload` APIs

The `thread_rpc_alloc_*_payload` APIs are consistent with the `thread_shm_type` enumerations, as seen in the `alloc_shm` function. However, the name and comments of `thread_rpc_alloc_payload` might be misleading, suggesting it is a universal API. To maintain backward compatibility, only the code comments have been updated for clarity.
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make header file lines and commit message lines be 80char/line max.

The commit header file is too long and would preferably start with core: thread:, maybe: core: thread: clarify thread_rpc_alloc_*_payload() inline description.

Please add your Signed-off-by tag in the commit message (see contribution).

@@ -244,7 +244,8 @@ bool thread_is_in_normal_mode(void);
bool thread_is_from_abort_mode(void);

/**
* Allocates data for payload buffers.
* Allocates data for payload buffers only shared with user space applications.
* Ensure consistency with the enumeration `THREAD_SHM_TYPE_APPLICATION`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That not entirely true: only is misleading. Maybe prefer Allocate data for payload buffers shared with a non-secure user space application.

Prefer drop the back quotes around THREAD_SHM_TYPE_APPLICATION.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants