DAOS-18304 ddb: Fix memory leaks, defer ordering, and pool handle corruption#18125
Open
DAOS-18304 ddb: Fix memory leaks, defer ordering, and pool handle corruption#18125
Conversation
When the pool was already open before ddb_run_feature() was called (i.e. the 'close' flag is false), resetting dc_poh to DAOS_HDL_INVAL and dc_write_mode to false unconditionally corrupted the caller's pool state. Guard these resets inside the 'if (close)' block so they only execute when this function itself opened the pool. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
- Feature(): C.CString(enable) and C.CString(disable) were passed directly to ddb_feature_string2flags() without ever being freed. Assign them to named variables so defer freeString() can release them. - DtxStat(): options.details was set before defer freeString(options.path), inverting the defer execution order. Move the defer immediately after the CString allocation. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
- helper_stat_open_modify_close_stat() overwrote tctx->dvt_poh with the temporary pool handle opened for the test and never restored it, leaving the suite-level pool handle invalid for all subsequent tests. Save and restore the original handle around the open/close sequence. - read_only_vs_write_mode_test() passed fs[FILE_STATE_PRE].digest as both arguments to assert_memory_equal(), comparing the pre-digest against itself and making the read-only assertion a no-op. Use fs[FILE_STATE_POST].digest as the second argument. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
6 tasks
knard38
changed the title
6 tasks
|
Ticket title is 'Add unit test to ddb go code' |
Fix clang format issue detected by CI. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
janekmi
approved these changes
May 1, 2026
…/daos-18304/patch-000
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
This PR is the first split of #17967, broken out at reviewers' request to make the review process easier. It contains unrelated code fixes found during the creation of the initial PR. The second split #18124 contains the main refactoring and will be reviewed separately.
Go layer
Feature():C.CString(enable)andC.CString(disable)were passed directly toddb_feature_string2flags()without ever being freed; assign them to named variables sodefer freeString()can release them.DtxStat():options.detailswas set beforedefer freeString(options.path), inverting the defer execution order; move the defer immediately after theCStringallocation.C layer
ddb_run_feature():dc_pohanddc_write_modewere reset unconditionally, corrupting the caller's pool state when the pool was already open before the call; guard both resets inside theif (close)block.helper_stat_open_modify_close_stat(): the function overwrotetctx->dvt_pohwith a temporary handle and never restored it, leaving the suite-level handle invalid for subsequent tests; save and restore the original handle around the open/close sequence.read_only_vs_write_mode_test():assert_memory_equal()usedfs[FILE_STATE_PRE].digestas both arguments, making the read-only assertion a no-op; usefs[FILE_STATE_POST].digestas the second argument.Steps for the author:
After all prior steps are complete: