[FEAT] WIP module decoder_isdb in lib_ccxr#1676
[FEAT] WIP module decoder_isdb in lib_ccxr#1676vatsalkeshav wants to merge 4 commits intoCCExtractor:masterfrom
decoder_isdb in lib_ccxr#1676Conversation
|
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 9e2a594...:
All tests passing on the master branch were passed completely. NOTE: The following tests have been failing on the master branch as well as the PR:
Check the result page for more info. |
1a64f79 to
dd06fe2
Compare
isdbs module in lib_ccxrisdbs module in lib_ccxr
0165690 to
2fa5da9
Compare
b3e38d4 to
6a4319d
Compare
isdbs module in lib_ccxrisdbs module in lib_ccxr
6a4319d to
2f6f178
Compare
isdbs module in lib_ccxrdecoder_isdb module in lib_ccxr
decoder_isdb module in lib_ccxrdecoder_isdb module in lib_ccxr
5bae956 to
ccdfeee
Compare
|
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit b62027a...:
All tests passing on the master branch were passed completely. NOTE: The following tests have been failing on the master branch as well as the PR:
Congratulations: Merging this PR would fix the following tests:
Check the result page for more info. |
decoder_isdb module in lib_ccxrdecoder_isdb in lib_ccxr
decoder_isdb in lib_ccxrdecoder_isdb in lib_ccxr
cfsmp3
left a comment
There was a problem hiding this comment.
Thank you for working on migrating the ISDB decoder to Rust! The overall structure looks good and the build succeeds.
However, I found a critical memory safety bug that needs to be fixed:
Use-after-free in reserve_buf() (functions_isdb.rs:59-80):
let mut new_buf = Vec::with_capacity(blen);
new_buf.extend_from_slice(unsafe { std::slice::from_raw_parts(text.buf, text.used) });
text.buf = new_buf.as_mut_ptr(); // Store pointer to Vec's buffer
text.len = blen;
// BUG: new_buf is dropped here, freeing the memory!
// text.buf now points to freed memory - use-after-free!The new_buf Vec goes out of scope and is deallocated, but text.buf still points to the freed memory. You correctly used std::mem::forget(buf) in allocate_text_node() to prevent this - the same pattern is needed here:
text.buf = new_buf.as_mut_ptr();
text.len = blen;
std::mem::forget(new_buf); // Add this line to prevent deallocation!Additionally, the old buffer (text.buf) should be freed before being replaced to avoid a memory leak. Something like:
// Free old buffer if it exists
if !text.buf.is_null() && text.len > 0 {
unsafe { drop(Vec::from_raw_parts(text.buf, text.used, text.len)); }
}
// Then allocate new buffer...This bug likely explains why the CI - linux sample platform tests are timing out - use-after-free can cause infinite loops, crashes, or memory corruption.
Other notes:
- PR has merge conflicts that need to be resolved
- Remove "WIP" from title when ready for final review
- Consider adding unit tests for the Rust implementation to catch issues like this
Once the memory bug is fixed and rebased, this should be ready for more detailed functional testing.
|
Sure I'll push some local changes after working on #1679 |
|
Closed due to inactivity, feel free to submit a new one (make sure it's based off master) when you are ready to resume work. |
|
covered in #2109 |
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
This PR migrates
/src/lib_ccx/ccx_decoders_isdb.cto rust.These changes have been made-
/src/rust/lib_ccxr/src/decoder_isdb/functions_isdb.rs, functions_common.rs/src/rust/lib_ccxr/src/decoder_isdb/structs_xds.rs,structs_isdb.rs,structs_ccdecode.rs(thanks @steel-bucket)/src/rust/lib_ccxr/src/decoder_isdb/exit_codes.rssrc/rust/src/libccxr_exports/isdb_exports.rs/src/lib_ccx/ccx_decoders_isdb.c