389-ds-base

#50480 Ticket 50445 - ldbm_config_*_set functions need to validate valuse in apply=0 mode
Closed by spichugi. Opened by lkrispen.
lkrispen/389-ds-base t50445  into  master

Download 50480.patch

Bug: Some config set functions do not perform the checks for passed values if
called in apply=0 mode.
There is a memory leak if setting the config values in ldbm_config_modify_entry_callback()
fails and reapply_mods was aready set.

Fix: to prevent the memory leak only modify the list of mods if all parameter
settings will succeed. this also requires that checks are correctly
execute in apply mode 0.
For all confi set functions which do checks do it also for apply_mode 0.

Reviewd by: ?

The patch looks good . ACK
The overall code looks complex would you mind to add a comment in the block 'if (reapply_mods) {' to clarify that it is the job of dse layer to apply the ignored attributes 'modifytimestamps'... and free them.

rebased onto f50f40182519e2d733fa7a98ebbd7da52b90f79c

rebased onto 80ac6e66c12e727c74b02a108b8e5f8052fbfca1

@tbordaz I added a comment, could you please verify if this does make sense and if the function does what the comment say

My understanding is that dse_callbacks will consume (apply/free) the MODs that related to them and keep (in SLAPI_MODIFY_MODS) the MODs that they ignore.
After dse_callbacks, the ignored MODs will be reapply to the postEntry. But as they were already applied before calling dse_callbacks, the dse_callbacks revert in the postEntry[MOD.attributname] = preEntry[MOD.attributename.
Like mentioned in dse.c I also do not know why we entry_apply_mods(postEntry, MODs). I guess it is to verify that it would be successful.
But then I do not understand why revert/reapply updates related to ignored attribute, why not just keeping the value in the postEntry.

Regarding your comment, I have a doubt regarding 'removed ignored attribute'. My understanding is that they are not removed but let in the MODs to be reapplied. Also this is the dse_callback that reset the orginal values in postEntry letting dse to reapply the ignored MODs.

I fully agree with your point. dse_callbacks should not know what will follow and if it is useless to revert postEntry and reapply later, it would help to clarify that code.

you are right about the comment of ignored attributes, they are removed from the entry and the only ones kept in the list of mods to be reapplied.

And I missed that you wanted the comments in the level of the caller. I will rethink and rewrite it

@lkrispen, sorry I did not want to be nit picker. The patch is good but it took us so much time to understand this part of code that a comment would be a great help for the next one to dig into that place.

I have looked and also agree the patch looks reasonable. Any thing outstanding before we merge @lkrispen and @tbordaz ?

@lkrispen - is this patch still relevant with the backend work you are doing? If yes, you have all the acks you need to merge it :-)

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3537

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.