Store the last added child in the PState for use in separator logic#889
Merged
stevedlawrence merged 1 commit intoapache:mainfrom Dec 9, 2022
Merged
Conversation
In some cases, separator logic needs knowledge about the most recently added child of an element. This is currently handled by examining the infoset and finding its last child element using the maybeMostRecentlyAddedChild function. The problem with this approach is that the InfosetWalker is allowed to "release" elements from the infoset that it thinks are no longer needed. There isn't a good way to tell the infoset that these last children are potentially still needed for separator logic, and so it could actually release them prior to the separator logic trying to access them, which leads to a null pointer exception. To fix this, this modifies the PState to add a slot for the last modified child of the current infoset element, and modifies the ElementParser's to set this state appropriately. This way, the InfosetWalker is free to remove elements as it normally does, but the last child is still available when needed. This also removes the maybeMostRecentlyAddedChild functions since this kind of access to the infoset can lead to null pointers. Also modifies the SAXInfosetOutputter to check isNilled correctly, which supports checking if complex elements are nilled without being final yet. For similar reasons, modifies the InfosetWalker so that it does not walk into Complex elements if there is a chance that it could be nilled and we might not be sure. DAFFODIL-2755
mbeckerle
approved these changes
Dec 8, 2022
Contributor
mbeckerle
left a comment
There was a problem hiding this comment.
+1 Very clean and small change.
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.
In some cases, separator logic needs knowledge about the most recently added child of an element. This is currently handled by examining the infoset and finding its last child element using the maybeMostRecentlyAddedChild function.
The problem with this approach is that the InfosetWalker is allowed to "release" elements from the infoset that it thinks are no longer needed. There isn't a good way to tell the infoset that these last children are potentially still needed for separator logic, and so it could actually release them prior to the separator logic trying to access them, which leads to a null pointer exception.
To fix this, this modifies the PState to add a slot for the last modified child of the current infoset element, and modifies the ElementParser's to set this state appropriately. This way, the InfosetWalker is free to remove elements as it normally does, but the last child is still available when needed.
This also removes the maybeMostRecentlyAddedChild functions since this kind of access to the infoset can lead to null pointers.
Also modifies the SAXInfosetOutputter to check isNilled correctly, which supports checking if complex elements are nilled without being final yet. For similar reasons, modifies the InfosetWalker so that it does not walk into Complex elements if there is a chance that it could be nilled and we might not be sure.
DAFFODIL-2755