lib_manager: llext_manager: Add const to variables and function parameters cleanup #8898
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
softwarecki
requested review from
lyakh,
lgirdwood,
kv2019i,
jxstelter and
pjdobrowolski
softwarecki
force-pushed
the
manager-cleanup
branch
2 times, most recently
from
589c508
to
122e9c4
Compare
softwarecki
requested review from
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
kv2019i
reviewed
Mar 6, 2024
lyakh
reviewed
Mar 6, 2024
lyakh
previously requested changes
Mar 7, 2024
lgirdwood
reviewed
Mar 8, 2024
jxstelter
approved these changes
Mar 8, 2024
softwarecki
force-pushed
the
manager-cleanup
branch
from
122e9c4
to
730fc12
Compare
softwarecki
force-pushed
the
manager-cleanup
branch
from
730fc12
to
82d63a6
Compare
lyakh
dismissed
their stale review
the missing lib_manager_free_module() instance got fixed, but component_id still conflicts with the component API IMHO
Added a const modifier to the module manifest variable in the lib_manager_store_library function to prevent it from being accidentally modified. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
The preload_page_count value from the loadable library manifest is used to determine size of allocated memory buffer. If this value is smaller than the manifest size, it will lead to memory corruption. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Removed pointer to a library manifest from the parameters list of the lib_manager_register_module function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Instead of a pointer to a component driver and ipc config, the lib_manager_free_module and llext_manager_free_module functions now gets component id. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
…nters To prevent unintentional modification of a loadable library manifest, a const modifier has been added to all variables pointing to it. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Added a const modifier to the ipc_config parameter passed to functions lib_manager_allocate_module and llext_manager_allocate_module. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Removed unused desc parameter from llext_manager_load_module and llext_manager_unload_module functions. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
softwarecki
force-pushed
the
manager-cleanup
branch
from
82d63a6
to
5950a18
Compare
kv2019i
approved these changes
Mar 11, 2024
lyakh
previously requested changes
Mar 12, 2024
There was a problem hiding this comment.
In principle I like adding const to pointers in general and to pointer function arguments in particular. But I think this has to be improved:
- I disagree with the "component ID" part
- in one location an existing function should be used instead of open-coding
- let's do that after we revert the
lib_manager_free_module()as per bc79c69
4 tasks
@softwarecki ok, you're right: the revert only touches modules.c and this PR hardly touches it. Let me remove my "request for change" then, but I still stand by my other two comments. Besides, as of yesterday the QB seems to be broken, which would also fail this PR if you update it #8933 (comment) |
lyakh
dismissed
their stale review
my main objection turned out to be invalid, the revert should still be possible on top of this PR
|
The DSP panic in https://sof-ci.01.org/sofpr/PR8898/build3297/devicetest/index.html?model=MTLP_SDW_AIOC&testcase=check-playback-10sec is a known fail and tracked in #8908 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a const modifier to the module manifest variable in the lib_manager_store_library function to prevent it from being accidentally modified.
Added verification of preload_page_count from module manifest. The preload_page_count value from the loadable library manifest is used to determine size of allocated memory buffer. If this value is smaller than the manifest size, it will lead to memory corruption.
Removed pointer to a library manifest from the parameters list of the lib_manager_register_module function.
Instead of a pointer to a component driver and ipc config, the lib_manager_free_module and llext_manager_free_module functions now gets component id.
To prevent unintentional modification of a loadable library manifest, a const modifier has been added to all variables pointing to it.
Added a const modifier to the ipc_config parameter passed to functions lib_manager_allocate_module and llext_manager_allocate_module.
Removed unused desc parameter from llext_manager_load_module and llext_manager_unload_module functions.