Consolidate CallbackVector cleanup into free_cbv()#288
Draft
toddr-bot wants to merge 1 commit into
Draft
Conversation
Expand free_cbv() to release all SV handler fields (with null guards) and replace the duplicated inline cleanup in XML_ParserFree with a single call to free_cbv(). This eliminates a maintenance trap where adding a new SV field to CallbackVector required updating two independent cleanup sites. The self_sv null guard ensures correctness for both paths: - Early-abort in XML_ParserCreate (self_sv is set, freed by free_cbv) - Normal teardown via XML_ParserFree (self_sv already NULL from Release) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
==========================================
+ Coverage 76.40% 77.06% +0.66%
==========================================
Files 1 1
Lines 1102 1112 +10
Branches 346 352 +6
==========================================
+ Hits 842 857 +15
+ Misses 52 45 -7
- Partials 208 210 +2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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
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.
What
Single-owner teardown:
free_cbv()now handles allCallbackVectorfields, andXML_ParserFreedelegates to it instead of duplicating the cleanup inline.Why
Adding a new SV field to
CallbackVectorpreviously required updating two independent cleanup sites —free_cbv()(for early-abort inParserCreate) and the inline block inXML_ParserFree(for normal teardown). Missing either one leaks memory. This consolidation makes the struct self-cleaning through one function.How
free_cbv()with null-guardedSvREFCNT_decfor every SV handler field (recstring,start_svthroughendcd_sv).self_svso the function is safe for both paths: early-abort (self_sv set) and normal teardown (self_sv already NULL fromXML_ParserRelease).XML_ParserFreewithfree_cbv(cbv).XML_ParserCreateare unaffected —Newxzzeroes all SV fields, andSvREFCNT_dec(NULL)/Safefree(NULL)are safe no-ops.Testing
Full test suite passes (
make test— all tests OK).🤖 Generated with Claude Code
Quality Report
Changes: 1 file changed, 64 insertions(+), 68 deletions(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline