CFE-4742: Fixed simulate mode reporting both removal and install of the same package - #6332
Open
djbclark wants to merge 2 commits into
Open
CFE-4742: Fixed simulate mode reporting both removal and install of the same package#6332djbclark wants to merge 2 commits into
djbclark wants to merge 2 commits into
Conversation
…ckage In DiffPkgOperations() and ManifestPkgOperations(), inserting an install message into the installed/present map set name_arch to NULL because the map took ownership of it as a key. The MapRemove() call that cancels a previous removal message ran after that, so it looked up a NULL key and matched nothing. The cancellation was silently skipped exactly when an install message had been inserted: a package recorded as removed and then installed was reported as both removed and installed. In debug builds a NULL key also fails an assertion in StringHash() once the map outgrows the array-map stage. Moved the cancellation above the code that hands name_arch to the map. Changelog: Title Ticket: CFE-4742 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
djbclark
added a commit
to frdminc/tendcf
that referenced
this pull request
Aug 22, 2026
…re#6332) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
larsewi
reviewed
Aug 25, 2026
larsewi
left a comment
larsewi
left a comment
Contributor
There was a problem hiding this comment.
The core fix is correct. In both functions MapInsert takes ownership of name_arch followed by setting it to NULL. Hence, the following MapRemove was looking up a NULL key.
Co-authored-by: Lars Erik Wik <53906608+larsewi@users.noreply.github.com>
Contributor
|
Could you please squash the last the last commit? And while you're at it, feel free to add a signed-of-by trailer with |
Contributor
|
@cf-bottom Jenkins please :) |
larsewi
added
the
cherry-pick?
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14576/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14576/ |
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.
Ticket: https://northerntech.atlassian.net/browse/CFE-4742
In
DiffPkgOperations()andManifestPkgOperations(), inserting an install message into the installed/present map setname_archto NULL because the map took ownership of it as a key. TheMapRemove()call that cancels a previous removal message ran after that, so it looked up a NULL key and matched nothing: a package recorded as removed and then installed was reported as both removed and installed. Moved the cancellation above the code that handsname_archto the map.The regression test feeds
r,foo,,+i,foo,1.2.3,records to both functions: it fails before the fix and passes after, verified on x86_64 Linux (CI), Ubuntu 24.04 arm64 (dash), and macOS arm64.AI-assisted. Full disclosure in the ticket: I reviewed the fix's logic with AI guidance and saw no issues, but I cannot personally vouch for the C itself — the correctness case rests on the discriminating regression test and CI above. If that does not meet the bar for human review, closing this is a fine outcome and the documented finding stands.