Repository navigation
[FIX] Treat NULL as empty in the Levenshtein FFI exports - #2391
Merged
Merged
Conversation
C represents an empty compare buffer as (NULL, 0): telxcc resets ucs2_buffer_cur to NULL after each buffer switch, and fuzzy_memcmp() passes it on to levenshtein_dist() when a page carried no text. slice::from_raw_parts requires a non-null pointer even for a zero length, so ccxr_levenshtein_dist() was undefined behaviour there. Debug builds abort on the precondition check (e.g. --tpages-all on a teletext sample); release builds don't check it. Build the slices through a helper that maps NULL to an empty slice, and use it in ccxr_levenshtein_dist_char() too, which has the same hazard.
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 raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Repro instructions:
Build with
./build -debugand runccextractor b8c55aa2e9f27f1355c3b7848e513474a78d1efb56973134fd9f665e6c86a2ef.mpg --tpages-all -o out.srt(teletext sample). It aborts:Release builds don't check the precondition, so there it is silent undefined behaviour.
C represents an empty compare buffer as
(NULL, 0):telxcc.cresetsucs2_buffer_curto NULL after every buffer switch, andprocess_page()→fuzzy_memcmp()passes it tolevenshtein_dist()when a page carried no text. That's fine in C, butslice::from_raw_partsrequires a non-null pointer even for a zero length, soccxr_levenshtein_dist()was UB.The slices are now built through a helper that maps NULL to an empty slice;
ccxr_levenshtein_dist_char()uses it too, since it has the same hazard. The Safety docs say NULL denotes an empty slice.Verified:
--teletext,--tpages-allsrt/webvtt across 29 samples): release output and exit codes byte-identical to master.cargo test,cargo fmt --checkandcargo clippy -D warningsclean in both crates.