Skip to content

Conversation

@cfsmp3
Copy link
Contributor

@cfsmp3 cfsmp3 commented Dec 17, 2025

Summary

  • Fixed dangling pointer bug in Rust FFI copy_from_rust() function that caused memory corruption
  • Added missing settings_dtvcc.timing = NULL initialization in C's init_options()

Problem

The to_ctype() implementations for DecoderDtvccSettings and Decoder608Settings were creating temporaries on the stack and returning pointers to them:

// In to_ctype():
report: if let Some(value) = self.report {
    &mut value.to_ctype()  // ← Creates temporary, returns dangling pointer!
} else { ... },
timing: &mut self.timing.to_ctype(),  // ← Same issue

When copy_from_rust() called to_ctype(), these dangling pointers were written to the C ccx_options struct. Later when C code tried to use settings_dtvcc.report or settings_dtvcc.timing, it accessed invalid memory.

Valgrind errors before this fix:

Invalid write of size 4
   at 0x1D44ED: dtvcc_init
   Address 0x... is on thread 1's stack, 4016 bytes below stack pointer

Invalid read of size 1
   at 0x32F426: ccx_rust::common::copy_to_rust
   Address 0x... is on thread 1's stack, 1280 bytes below stack pointer

Solution

Preserve the original C-managed pointers instead of overwriting them with dangling pointers:

// Save original C pointers
let saved_report = (*ccx_s_options).settings_dtvcc.report;
let saved_timing = (*ccx_s_options).settings_dtvcc.timing;

// Apply other fields from to_ctype()
(*ccx_s_options).settings_dtvcc = options.settings_dtvcc.to_ctype();

// Restore the C-managed pointers
(*ccx_s_options).settings_dtvcc.report = saved_report;
(*ccx_s_options).settings_dtvcc.timing = saved_timing;

Test plan

  • Verified all 21 Teletext tests pass (tests 63-83)
  • Verified test 18 and 19 pass with exit code 0
  • Verified valgrind no longer reports "Invalid read/write" errors
  • Error summary reduced from 131,099 errors (15 contexts) to 131,074 errors (4 contexts) - the remaining warnings are unrelated uninitialized memory issues

🤖 Generated with Claude Code

The `to_ctype()` implementations for `DecoderDtvccSettings` and
`Decoder608Settings` were creating temporaries on the stack and
returning pointers to them. These pointers became dangling after
the function returned, causing memory corruption when
`copy_from_rust()` was called.

This fix:
- Preserves the original C-managed `report` and `timing` pointers
  in `copy_from_rust()` instead of overwriting them with dangling
  pointers to temporaries
- Adds explicit `settings_dtvcc.timing = NULL` initialization in
  `init_options()` for completeness

Before this fix, valgrind reported:
- "Invalid write of size 4" in `dtvcc_init` (4016 bytes below stack
   pointer)
- "Invalid read" errors in `copy_to_rust` / `DecoderDtvccSettings::
   from_ctype`

After this fix, these critical memory corruption errors are resolved.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@cfsmp3 cfsmp3 merged commit 588ad52 into master Dec 17, 2025
29 of 31 checks passed
@cfsmp3 cfsmp3 deleted the fix/rust-ffi-dangling-pointers branch December 19, 2025 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants