Fix FFI allocator mismatch by properly handling Rust/C ownership

src/rust/src/decoder/mod.rs

@@ -24,8 +24,10 @@ const CCX_DTVCC_MAX_PACKET_LENGTH: u8 = 128;
 const CCX_DTVCC_NO_LAST_SEQUENCE: i32 = -1;
 const CCX_DTVCC_SCREENGRID_ROWS: u8 = 75;
 const CCX_DTVCC_SCREENGRID_COLUMNS: u8 = 210;
-const CCX_DTVCC_MAX_ROWS: u8 = 15;
-const CCX_DTVCC_MAX_COLUMNS: u8 = 32 * 2;
+pub(crate) const CCX_DTVCC_MAX_ROWS: u8 = 15;
+pub(crate) const CCX_DTVCC_MAX_COLUMNS: u8 = 32 * 2;
+
+pub use window::{alloc_row, dealloc_row};
 
 /// Context required for processing 708 data
 pub struct Dtvcc<'a> {

src/rust/src/decoder/window.rs

@@ -15,6 +15,29 @@ use std::{
     ptr::copy_nonoverlapping,
 };
 
+/// Get the layout for a single caption row
+fn row_layout() -> Layout {
+    Layout::array::<dtvcc_symbol>(CCX_DTVCC_MAX_COLUMNS as usize).expect("row layout calculation")
+}
+
+/// Allocate a new zero-initialized caption row
+///
+/// # Safety
+/// The returned pointer must be freed using `dealloc_row`
+pub unsafe fn alloc_row() -> *mut dtvcc_symbol {
+    alloc_zeroed(row_layout()) as *mut dtvcc_symbol
+}
+
+/// Deallocate a caption row
+///
+/// # Safety
+/// `ptr` must have been allocated by `alloc_row` or `alloc_zeroed` with `row_layout`
+pub unsafe fn dealloc_row(ptr: *mut dtvcc_symbol) {
+    if !ptr.is_null() {
+        dealloc(ptr as *mut u8, row_layout());
+    }
+}
+
 use super::timing::get_time_str;
 use super::{
     CCX_DTVCC_MAX_COLUMNS, CCX_DTVCC_MAX_ROWS, CCX_DTVCC_SCREENGRID_COLUMNS,
@@ -161,23 +184,15 @@ impl dtvcc_window {
     pub fn clear_row(&mut self, row_index: usize) {
         if is_true(self.memory_reserved) {
             unsafe {
-                let layout = Layout::array::<dtvcc_symbol>(CCX_DTVCC_MAX_COLUMNS as usize);
-                if let Err(e) = layout {
-                    error!("clear_row: Incorrect Layout, {e}");
-                } else {
-                    let layout = layout.unwrap();
-                    // deallocate previous memory
-                    if !self.rows[row_index].is_null() {
-                        dealloc(self.rows[row_index] as *mut u8, layout);
-                    }
-
-                    // allocate new zero initialized memory
-                    let ptr = alloc_zeroed(layout);
-                    if ptr.is_null() {
-                        error!("clear_row: Not enough memory",);
-                    }
-                    self.rows[row_index] = ptr as *mut dtvcc_symbol;
+                // deallocate previous memory
+                dealloc_row(self.rows[row_index]);
+
+                // allocate new zero initialized memory
+                let ptr = alloc_row();
+                if ptr.is_null() {
+                    error!("clear_row: Not enough memory",);
                 }
+                self.rows[row_index] = ptr;
             }
             for col in 0..CCX_DTVCC_MAX_COLUMNS as usize {
                 // Set pen color and attributes to default value

src/rust/src/demuxer/common_types.rs

@@ -1,13 +1,9 @@
 use crate::bindings::{lib_ccx_ctx, list_head};
+use crate::ffi_alloc;
 use lib_ccxr::common::{Codec, Decoder608Report, DecoderDtvccReport, StreamMode, StreamType};
 use lib_ccxr::time::Timestamp;
-use std::os::raw::c_void;
 use std::ptr::null_mut;
 
-extern "C" {
-    fn free(ptr: *mut c_void);
-}
-
 // Size of the Startbytes Array in CcxDemuxer - const 1MB
 pub(crate) const ARRAY_SIZE: usize = 1024 * 1024;
 
@@ -284,19 +280,15 @@ impl Drop for CcxDemuxer<'_> {
         // Free all non-null PSIBuffer pointers.
         // These are freed using C's free to be compatible with memory that might be allocated by C.
         for ptr in self.pid_buffers.drain(..) {
-            if !ptr.is_null() {
-                unsafe {
-                    free(ptr as *mut c_void);
-                }
+            unsafe {
+                ffi_alloc::c_free(ptr);
             }
         }
         // Free all non-null PMTEntry pointers.
         // These are freed using C's free to be compatible with memory that might be allocated by C.
         for ptr in self.pids_programs.drain(..) {
-            if !ptr.is_null() {
-                unsafe {
-                    free(ptr as *mut c_void);
-                }
+            unsafe {
+                ffi_alloc::c_free(ptr);
             }
         }
     }

src/rust/src/demuxer/stream_functions.rs

@@ -1,5 +1,6 @@
 use crate::bindings::ccx_demuxer;
 use crate::demuxer::common_types::{CcxDemuxer, CcxStreamMp4Box, STARTBYTESLENGTH};
+use crate::ffi_alloc;
 use crate::file_functions::file::{buffered_read_opt, return_to_buffer};
 use crate::libccxr_exports::demuxer::{alloc_new_demuxer, copy_demuxer_from_rust_to_c};
 use cfg_if::cfg_if;
@@ -145,7 +146,7 @@ unsafe fn detect_stream_type_common(ctx: &mut CcxDemuxer, ccx_options: &mut Opti
         copy_demuxer_from_rust_to_c(demuxer, ctx);
         let private = ccx_gxf_init(demuxer);
         ctx.private_data = private as *mut core::ffi::c_void;
-        drop(Box::from_raw(demuxer));
+        ffi_alloc::c_free(demuxer);
     }
 
     // WTV check
@@ -269,7 +270,7 @@ unsafe fn detect_stream_type_common(ctx: &mut CcxDemuxer, ccx_options: &mut Opti
             let private = ccx_mxf_init(demuxer);
             ctx.private_data = private as *mut core::ffi::c_void;
         }
-        drop(Box::from_raw(demuxer));
+        ffi_alloc::c_free(demuxer);
     }
 
     // Still not found

src/rust/src/encoder/simplexml.rs

@@ -1,6 +1,7 @@
 use crate::bindings::{cc_subtitle, ccx_encoding_type_CCX_ENC_ASCII, eia608_screen, encoder_ctx};
 use crate::encoder::ccxr_get_str_basic;
 use crate::encoder::common::write_raw;
+use crate::ffi_alloc;
 use lib_ccxr::common::CCX_DECODER_608_SCREEN_WIDTH;
 use lib_ccxr::info;
 use std::os::raw::{c_int, c_void};
@@ -79,8 +80,7 @@ pub fn write_cc_bitmap_as_simplexml(sub: &mut cc_subtitle, _context: &mut encode
 
     if !sub.data.is_null() {
         unsafe {
-            let data_box = Box::from_raw(sub.data as *mut u8);
-            drop(data_box);
+            ffi_alloc::c_free(sub.data);
         }
         sub.data = ptr::null_mut();
     }

src/rust/src/ffi_alloc.rs

@@ -0,0 +1,18 @@
+use std::os::raw::c_void;
+
+extern "C" {
+    fn free(ptr: *mut c_void);
+}
+/// Frees memory allocated by C code (malloc/calloc)
+///
+/// # Safety
+///
+/// The pointer must have been allocated by C's malloc/calloc/realloc and
+/// not yet freed. Calling this function with a pointer that was not
+/// allocated by the C allocator is undefined behavior.
+#[inline]
+pub unsafe fn c_free<T>(ptr: *mut T) {
+    if !ptr.is_null() {
+        free(ptr as *mut c_void);
+    }
+}

src/rust/src/lib.rs

@@ -21,6 +21,7 @@ pub mod decoder;
 pub mod demuxer;
 pub mod encoder;
 pub mod es;
+pub mod ffi_alloc;
 pub mod file_functions;
 #[cfg(feature = "hardsubx_ocr")]
 pub mod hardsubx;
@@ -272,10 +273,8 @@ pub extern "C" fn ccxr_dtvcc_free(dtvcc_ptr: *mut std::ffi::c_void) {
             for window in decoder.windows.iter() {
                 if is_true(window.memory_reserved) {
                     for row_ptr in window.rows.iter() {
-                        if !row_ptr.is_null() {
-                            unsafe {
-                                drop(Box::from_raw(*row_ptr));
-                            }
+                        unsafe {
+                            decoder::dealloc_row(*row_ptr);
                         }
                     }
                 }

src/rust/src/libccxr_exports/demuxer.rs

@@ -18,13 +18,14 @@ const POISON_PTR_PATTERN: usize = 0xcdcdcdcdcdcdcdcd;
 const POISON_PTR_PATTERN: usize = 0xcdcdcdcd;
 
 extern "C" {
-    fn activity_input_file_closed();
-    fn close(fd: c_int) -> c_int;
-    fn malloc(size: usize) -> *mut c_void;
-    fn free(ptr: *mut c_void);
-    fn calloc(nmemb: usize, size: usize) -> *mut c_void;
+    pub(crate) fn activity_input_file_closed();
+    pub(crate) fn close(fd: c_int) -> c_int;
+    pub(crate) fn malloc(size: usize) -> *mut c_void;
+    pub(crate) fn calloc(nmemb: usize, size: usize) -> *mut c_void;
 }
 
+use crate::ffi_alloc;
+
 pub fn copy_c_array_to_rust_vec(
     c_bytes: &[u8; crate::demuxer::common_types::ARRAY_SIZE],
 ) -> Vec<u8> {
@@ -110,7 +111,7 @@ pub unsafe fn copy_demuxer_from_rust_to_c(c_demuxer: *mut ccx_demuxer, rust_demu
         // We also check for POISON_PTR_PATTERN for safety in debug builds.
         if !c.PID_buffers[i].is_null() && c.PID_buffers[i] as usize != POISON_PTR_PATTERN {
             unsafe {
-                free(c.PID_buffers[i] as *mut c_void);
+                ffi_alloc::c_free(c.PID_buffers[i]);
                 c.PID_buffers[i] = std::ptr::null_mut();
             }
         }
@@ -152,7 +153,7 @@ pub unsafe fn copy_demuxer_from_rust_to_c(c_demuxer: *mut ccx_demuxer, rust_demu
         // SAFETY: We use C's free to be compatible with memory that might be allocated by C.
         if !c.PIDs_programs[i].is_null() && c.PIDs_programs[i] as usize != POISON_PTR_PATTERN {
             unsafe {
-                free(c.PIDs_programs[i] as *mut c_void);
+                ffi_alloc::c_free(c.PIDs_programs[i]);
                 c.PIDs_programs[i] = std::ptr::null_mut();
             }
         }

src/rust/src/parser.rs

@@ -1663,6 +1663,7 @@ pub mod tests {
         common::{MkvLangFilter, OutputFormat, SelectCodec, StreamMode, StreamType},
         util::{encoding::Encoding, log::DebugMessageFlag},
     };
+    use serial_test::serial;
 
     /// # Safety
     ///
@@ -2318,18 +2319,21 @@ pub mod tests {
     }
 
     #[test]
+    #[serial]
     fn test_buffersize_with_k_suffix() {
         let (_, _) = parse_args(&["--buffersize", "64K"]);
         assert_eq!(get_file_buffer_size(), 64 * 1024);
     }
 
     #[test]
+    #[serial]
     fn test_buffersize_with_m_suffix() {
         let (_, _) = parse_args(&["--buffersize", "2M"]);
         assert_eq!(get_file_buffer_size(), 2 * 1024 * 1024);
     }
 
     #[test]
+    #[serial]
     fn test_buffersize_with_numeric_value() {
         let (_, _) = parse_args(&["--buffersize", "8192"]);
         assert_eq!(get_file_buffer_size(), 8192);