#1977 Segfault at startup when Memory Expansion is enabled in xplus4

[SukkoPera]

When a "Memory expansion hack" is enabled under Settings -> Model in xplus4, it works fine, but if the configuration is saved, the emulator is not able to start up again:

sukko@insight ~ $ xplus4 
Loading system file `/usr/share/vice/PRINTER/mps803-D7811G-111-U32053A.bin'.
Reading configuration file `/home/sukko/.config/vice/vicerc'.
CSORY 256KiB expansion installed.
Segmentation fault

Removing the "MemoryHack=<n>" line from vicerc allows xplus4 to start up correctly again.</n>

This bug was already present in 3.7 at least and it is still present in 3.8.

[compyx]

My VICE build has debugging enabled, and running xplus4 -default, then enabling CSORY 256K, saving settings and running xplus4 gives us an assertion:

Reading configuration file `/home/compyx/.config/vice/vicerc'.
CSORY 256KiB expansion installed.
xplus4: ../../../../trunk/vice/src/plus4/plus4io.c:451: io_source_unregister: Assertion `device != NULL' failed.
Aborted

[compyx]

After some digging I found that plus4io_init() is called after the "MemoryHack" resource initialization is called, which in turn calls the setters for the various memory hacks, which call plus4_pio_init(-1). That is what triggers the assertion, plus4_pio_init() tries to unregister an IO source that isn't yet registered (and the assertion indicates we want to do that, not just ignore a NULL value).

So moving the call to plus4io_init() into machine_resources_init(), before the call to plus4_resources_init() makes sure the IO sources a registered so the memhacks code can properly unregister them.

I however have zero knowledge of the Plus4, let alone its memory hacks/IO, so I'll post a diff here with the proposed change:

Index: src/plus4/plus4.c
===================================================================
--- src/plus4/plus4.c   (revision 44941)
+++ src/plus4/plus4.c   (working copy)
@@ -464,6 +464,13 @@
         init_resource_fail("traps");
         return -1;
     }
+
+    /* Initialize the machine specific I/O */
+    /* We need to call this before `plus4_resources_init()` otherwise the
+     * "MemoryHack" resource setter tries to unregister an IO source that isn't
+     * registered yet. */
+    plus4io_init();
+
     if (plus4_resources_init() < 0) {
         init_resource_fail("plus4");
         return -1;
@@ -929,9 +936,6 @@

     machine_drive_stub();

-    /* Initialize the machine specific I/O */
-    plus4io_init();
-
     return 0;
 }

[SukkoPera]

Patch applied, did a quick test and everything seems to be working fine.

I'll test more, but thanks in the meantime :).

[compyx]

Alright, thanks for checking. I'll wait for someone with more knowledge of the IO sources system to check that this patch doesn't inadvertently trigger other issues.

[Marco van den Heuvel]

it's correct

[compyx]

Yay! Applied in r44947.