search

tonioni / WinUAE

WinUAE Amiga emulator
http://www.winuae.net/
545 stars 87 forks source link

Crash with specific Akiko config - NULL pointer #217

Closed Waccoon closed 2 years ago

Waccoon commented 2 years ago

The config settings allow the 3 major CD32 features to be enabled independently: CD-ROM support, chunky-to-planar, and nvRAM.

If CD support is disabled and nvRAM is enabled, this leads to the Akiko ID showing up in the memory map, but the nvRAM is not initialized and "cd32_eeprom" is NULL. If the nvRAM registers are touched, this crashes WinUAE with a NULL pointer exception. This happens regardless of whether an ".nvr" file has been declared or not.

I can fix this, but I'd like some input as to the preferred way to handle it. CD support is required to properly save the state of Akiko, so if the CD is disabled, restoring a savestate while nvRAM is being accessed or C2P is in progress would result in corruption of data. I assume the only reason why these settings can be enabled independently is for debugging and diagnostic work. So, what would be the best fix?

1.) If CD support is off, nvRAM and C2P will be disabled. The Akiko ID and registers will still be visible, but hitting the registers does nothing.

2.) If CD support is off, nvRAM will be read-only and C2P will work as normal. This prevents corruption of the nvRAM if a savestate is loaded, as the I2C direction register cannot be restored unless the CD control registers are saved as well. The loss of C2P state would probably only result in a minor glitch in Amiga programs, unlikely to be dangerous or result in data loss.

3.) Something else.

I've been playing around with C2P features without the "full" Akiko being available, so I think it's worth keeping the independent settings, and would prefer option #2. I believe only changes to "akiko.cpp" are necessary to fix this. Any thoughts?

tonioni commented 2 years ago

I pushed quick update that should fix this without introducing new restrictions.

rofl0r commented 2 years ago

for anyone else reading this, the mentioned fix is most likely https://github.com/tonioni/WinUAE/commit/6d0ffcb4a81eb6ff5dbbdeef9042940b2b2eff6e?w=1

Waccoon commented 2 years ago

Seems to be working fine now with all combinations. Thanks.