Use RapidXMLParser to resolve LibXMLParser::xmlCleanupParser crash #2
Conversation
WalkthroughThis pull request updates the file exclusion settings for the Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
svgnative.podspec (1)
61-61: 💡 Verification agent🧩 Analysis chain
Verify XML library dependency with RapidXMLParser
I notice that even after switching to RapidXMLParser, the project still includes 'xml2' as a library dependency. This is worth verifying.
🏁 Script executed:
#!/bin/bash # Check if RapidXMLParser relies on libxml2 echo "Checking RapidXMLParser.cpp for libxml2 dependencies..." find svg-native-viewer -name "RapidXMLParser.cpp" | xargs cat | grep -n "libxml" || echo "No direct libxml references found in RapidXMLParser.cpp" echo "Checking RapidXMLParser.cpp for includes..." find svg-native-viewer -name "RapidXMLParser.cpp" | xargs cat | grep -n "#include" || echo "No includes found in RapidXMLParser.cpp"Length of output: 589
XML Library Dependency in svgnative.podspec
Based on the grep checks in
RapidXMLParser.cpp, there are no references to "libxml" or any#includestatements suggesting that RapidXMLParser relies on libxml2. Unless another part of the project still depends on xml2, it appears that the'xml2'entry in thes.librarieslist may no longer be necessary. Please verify that no other modules require libxml2 before removing it from the podspec.
- Confirmed: No direct "libxml" string found in RapidXMLParser.cpp.
- Confirmed: No
#includestatements indicating libxml2 dependency.
🧹 Nitpick comments (2)
Package.swift (1)
30-30: Change looks good - switching to RapidXMLParser implementationThis change excludes LibXMLParser.cpp instead of RapidXMLParser.cpp, effectively switching the XML parser implementation to use RapidXMLParser. This aligns with the PR objective to resolve the LibXMLParser::xmlCleanupParser crash mentioned in issue #14.
Consider adding a comment explaining why this parser change was made, to provide context for future maintenance:
exclude: ["svgnative/src/win", "svgnative/src/xml/ExpatXMLParser.cpp", + // Using RapidXMLParser instead of LibXMLParser to resolve xmlCleanupParser crash (see issue #14) "svgnative/src/xml/LibXMLParser.cpp", "svgnative/src/ports/cairo",svgnative.podspec (1)
41-41: Consistent parser implementation change in podspec fileThe podspec file has been correctly updated to exclude LibXMLParser.cpp instead of RapidXMLParser.cpp, matching the change in Package.swift. This ensures consistency between Swift Package Manager and CocoaPods implementations.
As with Package.swift, consider adding a comment explaining the reason for this change:
s.exclude_files = ['svg-native-viewer/svgnative/src/xml/ExpatXMLParser.cpp', + # Using RapidXMLParser instead of LibXMLParser to resolve xmlCleanupParser crash (see issue #14) 'svg-native-viewer/svgnative/src/xml/LibXMLParser.cpp', 'svg-native-viewer/svgnative/src/ports/cairo',
|
"Awesome! I can confirm that the parser replacement solved my open regression test for multi-threaded SVG loading. The Here's the code snippet I used for testing: DispatchQueue.global(qos: .userInteractive).async {
autoreleasepool {
for _ in 0..<100 {
let url = svgURLs.randomElement()!
let _ = WebImage(url: url)
}
}
} |
Having this issue: SDWebImage/SDWebImageSVGNativeCoder#14
Seems to be resolved by using RapidXMLParser instead in my testing
Summary by CodeRabbit