Skip to content

Conversation

@cfsmp3
Copy link
Contributor

@cfsmp3 cfsmp3 commented Jan 10, 2026

Summary

Removes unused impl FromCType<*mut PMT_entry> for *mut PMTEntry which contained a critical bug: returning a pointer to a stack-allocated value (undefined behavior).

Problem

The deleted code did this:

let mut pmt_entry = PMTEntry { ... };
Some(&mut pmt_entry as *mut PMTEntry)  // BUG: dangling pointer!

When the function returns, pmt_entry is destroyed and the pointer becomes dangling.

Why delete instead of fix?

This code was never called anywhere in the codebase:

  • The actual usage in demuxer.rs:279 uses the value-returning variant: PMTEntry::from_ctype(*buffer_ptr)
  • That call site already wraps it properly: Box::into_raw(Box::new(PMTEntry::from_ctype(...)?))

Rather than fix dead buggy code (and introduce memory ownership complexity), just delete it.

Verification

  • Build passes ✓
  • All 265 Rust tests pass ✓
  • No code references this implementation

Related

Supersedes #1988 which attempted to fix this bug rather than delete the dead code.

🤖 Generated with Claude Code

Delete the unused `impl FromCType<*mut PMT_entry> for *mut PMTEntry`
implementation which had a critical bug: it returned a pointer to a
stack-allocated PMTEntry, causing undefined behavior (dangling pointer).

This code was never called anywhere in the codebase. The actual usage
in demuxer.rs uses the value-returning variant `FromCType<PMT_entry>
for PMTEntry` with explicit `Box::into_raw(Box::new(...))` wrapping,
which is the correct pattern.

Rather than fixing dead buggy code, just remove it.

Supersedes #1988

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 618df18...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 6/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 85/86
Teletext 21/21
WTV 13/13
XDS 34/34

NOTE: The following tests have been failing on the master branch as well as the PR:


This PR does not introduce any new test failures. However, some tests are failing on both master and this PR (see above).

Check the result page for more info.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 618df18...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 6/7
DVD 3/3
DVR-MS 2/2
General 25/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 81/86
Teletext 21/21
WTV 13/13
XDS 34/34

NOTE: The following tests have been failing on the master branch as well as the PR:

  • ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2..., Last passed:

    Test 7743

  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed:

    Test 7743

  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed:

    Test 7743

  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743


This PR does not introduce any new test failures. However, some tests are failing on both master and this PR (see above).

Check the result page for more info.

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.

3 participants