Skip to content

Fix PinscapePico crash when configured device is not connected#58

Merged
jsm174 merged 1 commit into
vpinball:masterfrom
dynajoe:fix/pspico-null-deref
Jul 6, 2026
Merged

Fix PinscapePico crash when configured device is not connected#58
jsm174 merged 1 commit into
vpinball:masterfrom
dynajoe:fix/pspico-null-deref

Conversation

@dynajoe

@dynajoe dynajoe commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Fix

UpdateOutputs indexes m_oldOutputValues (resized only on a successful connect) and lacked the if (!m_dev) return; guard the sibling KL25Z Pinscape has. Add it.

Why it matters

A configured-but-absent Pico leaves m_dev null, m_oldOutputValues empty, and GetNumberOfOutputs() returns 128 — so the first output change reads m_oldOutputValues[0] out of bounds → crash (e.g. a Cabinet.xml listing a Pico that's currently unplugged). C# binds the device in the Number setter (Devices.First(...) throws if the unit is absent), so a not-present Pico can never be configured and this state is unreachable.

Found by diffing against the DirectOutput C# source.

UpdateOutputs indexes m_oldOutputValues (resized only on a successful connect) and lacked the !m_dev guard the sibling Pinscape has. A configured-but-absent Pico leaves m_dev null, m_oldOutputValues empty, GetNumberOfOutputs()=128, so the first update reads m_oldOutputValues[0] out of bounds and crashes. C# binds the device in the Number setter (Devices.First throws if absent), so it cannot reach this state. Add the !m_dev guard, matching Pinscape::UpdateOutputs.
@jsm174

jsm174 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Hello. Thanks for the PR!

Converting this to draft as per the contributing rules. (I was behind in getting them added as was just done in Visual Pinball).

It's going to take time to review these. libdof is currently is use by several users with a variety of devices, and we have yet to receive issues about this.

Also this definitely needs thorough testing with the actual hardware.

@jsm174
jsm174 marked this pull request as draft July 5, 2026 18:07
@jsm174
jsm174 marked this pull request as ready for review July 6, 2026 21:25
@jsm174
jsm174 merged commit ba79dc5 into vpinball:master Jul 6, 2026
13 checks passed
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