Skip to content

Commit 6449910

Browse files
JoeMattclaude
andcommitted
fix: address Copilot review on PR #126
Three findings from Copilot's review of the leak fix: 1. src/m68000/m68kinterface.c had a redundant `extern struct instr * table68k;` -- already declared in readcpu.h. Removed. 2. m68k_done() reaching directly into table68k (owned by readcpu.c) was a brittle cross-module dependency. Moved the actual free into a new free_table68k() in readcpu.c (declared in readcpu.h alongside read_table68k). m68k_done now just calls free_table68k() + resets emulation_initialized. Ownership stays end-to-end inside the module that allocates. 3. test/test_hle_bios.c inline comment was too long for one line. Converted to a multi-line block above the call. Build sanity: make -j4 clean, c89-lint passes, dylib still gates 46 retro_* exports. Co-Authored-By: Claude Opus 4.7 <[email protected]>
1 parent a1e9ea0 commit 6449910

4 files changed

Lines changed: 25 additions & 9 deletions

File tree

src/m68000/m68kinterface.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,14 @@ void M68KDebugResume(void)
8181
// File-scope so m68k_done() below can reset it after freeing table68k.
8282
static uint32_t emulation_initialized = 0;
8383

84-
// Free process-lifetime allocations made by read_table68k(). Called
85-
// from JaguarDone() so ASAN runs see a clean shutdown.
86-
extern struct instr * table68k;
84+
// Symmetric counterpart to the lazy init in m68k_pulse_reset().
85+
// Defers the actual free to readcpu.c::free_table68k() so the table
86+
// is owned end-to-end by the module that allocates it (table68k is
87+
// declared in readcpu.h). Called from JaguarDone() so ASAN sees a
88+
// clean shutdown.
8789
void m68k_done(void)
8890
{
89-
if (table68k)
90-
{
91-
free(table68k);
92-
table68k = NULL;
93-
}
91+
free_table68k();
9492
emulation_initialized = 0;
9593
}
9694

src/m68000/readcpu.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,20 @@ static void build_insn(int insn)
922922
}
923923

924924

925+
/* Symmetric counterpart to read_table68k: free the opcode table built
926+
above. Idempotent (free(NULL) is a no-op). Called from
927+
m68k_done() in m68kinterface.c so process-lifetime ASAN runs see a
928+
clean shutdown. */
929+
void free_table68k(void)
930+
{
931+
if (table68k)
932+
{
933+
free(table68k);
934+
table68k = NULL;
935+
}
936+
}
937+
938+
925939
void read_table68k(void)
926940
{
927941
int i;

src/m68000/readcpu.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ extern struct instr {
112112
} *table68k;
113113

114114
extern void read_table68k(void);
115+
extern void free_table68k(void);
115116
extern void do_merges(void);
116117
extern int get_no_mismatches(void);
117118
extern int nr_cpuop_funcs;

test/test_hle_bios.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3172,7 +3172,10 @@ int main(int argc, char *argv[])
31723172

31733173
printf("\n=== Results: %d passed, %d failed ===\n", passes, fails);
31743174

3175-
p_retro_unload_game(); /* matches the second p_retro_load_game above; without this, JaguarDone() never runs and ASAN reports table68k + branch_condition_table as leaks */
3175+
/* Match the second p_retro_load_game (PAL) call above. Without
3176+
* this unload, JaguarDone() never runs for the PAL load and ASAN
3177+
* reports table68k + branch_condition_table as leaks. */
3178+
p_retro_unload_game();
31763179
p_retro_deinit();
31773180
dlclose(handle);
31783181
free(dummy_rom);

0 commit comments

Comments
 (0)