api: clean up failed encoder plugin initialization - #1904
Open
junkilee80 wants to merge 1 commit into
Open
Conversation
Signed-off-by: Junki Lee <junkilee80@gmail.com>
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.
Both
heif_context_get_encoder()variants store a new encoder wrapper in theoutput parameter before the plugin has finished initializing it. If the
plugin's
new_encoder()callback returns an error, the wrapper remainsallocated and the caller receives a non-null encoder from a failed call. Any
plugin state created before the error can be left behind as well.
Keep the wrapper local until initialization succeeds. On failure, delete it so
the existing plugin cleanup runs, set the output to null, and return the
original error unchanged.
The new test uses an encoder plugin that allocates state and then fails. It
checks both public APIs, the plugin's free callback, and the returned pointer.
The test fails 4 of 8 checks on the old code and passes with the patch, including
under ASan.
Local CTest result: 51 passed, 15 skipped. The existing
regiontest stillfails in this minimal-codec build; the same seven assertions fail on unmodified
master with the same configuration.
Found while reviewing RepoAudit output. PailGen suggested the cleanup pattern;
I reviewed the ownership path and added the regression test and validation.