core: OOM safety, sign-compare cleanup, dead-code removal, const-correctness
Omnibus cleanup pass. Build is clean on `make platform=unix` with
zero errors and zero warnings, including under
`-Wsign-compare -Wstrict-aliasing=2 -Wcast-align`. The
`-Wsign-compare` flag is now permanently enabled in
WARNING_DEFINES.
================================================================
A. FCEU_gmalloc no longer exit()s on OOM
================================================================
A libretro core must not call exit(): doing so tears down the entire
frontend. FCEU_gmalloc previously did exactly that on allocation
failure ("Doing a hard exit"). It now returns NULL with the same
diagnostic message.
Loader-level call sites that can fail their parent function (i.e.
return 0 from FDSLoad/iNESLoad/NSFLoad to refuse the cart) now check
the return value:
- ines.c: trainerpoo, ExtraNTARAM
- nsf.c: ExWRAM (both branches)
- fds.c: FDSBIOS, CHRRAM, FDSRAM
Mapper-level callers (~200 sites) are intentionally left as-is:
they live in void-returning Init/Power functions where graceful
failure isn't possible without a much larger restructuring. With
NULL returns those mappers will null-deref on first access, which
is contained to the core - the libretro frontend stays up. This is
strictly better than the previous behaviour of exit()ing the entire
frontend.
================================================================
B. -Wsign-compare cleanup (27 warnings -> 0)
================================================================
Surveyed every signed/unsigned comparison the compiler flagged and
fixed each one. Most fell into a few patterns:
Loop variables: changed `int x` to `uint32_t x` for loops over
uint32_t counts (TotalSides in fds.c, the trainer-copy loop in
6_8_12_17_561_562.c, soundOffset->SOUNDTS in 594.c).
scanline (signed) vs totalscanlines/normal_scanlines (unsigned)
in ppu.c: cast (unsigned)scanline at the comparison sites. The
scanline values are guaranteed >= 0 at every comparison site
(the loop initialises scanline=0 and increments).
rate_adjust macro in emu2413.c: the ?: was returning either
signed or unsigned depending on `rate`. Cast both branches to
uint32_t.
Mixed ?: branches in cartram.c (PRGRamSaveSize signed vs
WRAMSize unsigned): cast PRGRamSaveSize to uint32_t at use.
Other one-off casts in libretro.c, n625092.c, nsf.c, sound.c,
zapper.c, eeprom_93Cx6.c.
The Makefile change keeps -Wsign-compare permanently enabled so
new sign-compare bugs trip CI immediately.
Files touched: 594.c, 6_8_12_17_561_562.c, cartram.c,
eeprom_93Cx6.c, emu2413.c, n625092.c, fds.c, nsf.c, ppu.c,
sound.c, input/zapper.c, drivers/libretro/libretro.c, plus
Makefile.libretro.
================================================================
C. NULL-deref hardening on libretro callbacks
================================================================
retro_serialize and retro_unserialize now reject NULL data
pointers before passing them to memstream_set_buffer (which
would have null-deref'd inside the memstream code).
Other retro_* entry points that take pointers were already guarded
in earlier passes (retro_set_controller_port_device,
retro_get_memory_data, retro_get_memory_size, retro_load_game,
retro_cheat_set).
================================================================
D. Mapper coverage spot-check
================================================================
Wrote a heuristic scanner to find the savestate-load-array-index
bug class fixed in pass 1 (variable masked at write but unmasked
at restore -> OOB index). Scanner flagged 3 candidates across 424
mappers; manual review confirmed all three are false positives:
- sachen.c `cmd`: theoretical bug, but only one writer is wired
per game and that writer masks at use.
- unrom512.c `flash_state` and `latcha`: both already clamped
in StateRestore (added in pass 1).
The systematic bug class was thoroughly addressed in pass 1.
================================================================
E. Strip never-defined #ifdef symbols
================================================================
Surveyed every #ifdef symbol in the build and cross-checked
against #defines (in source and in build files). Removed code
gated on symbols that are never defined for any platform target:
- DEBUG_MAPPER (datalatch.c, 2 sites): a debug-print NROMWrite
handler. Dead.
- FRAMESKIP (fceu.h, driver.h, ppu.c, 4 sites): a legacy-FCEU
frameskip path. The libretro driver's `skip` argument to
FCEUI_Emulate is always 0, and FCEUI_FrameSkip is never
called by any libretro frontend. Removed the conditional
rendering branch in FCEUPPU_Loop along with the
FCEU_PutImageDummy declaration.
FRONTEND_SUPPORTS_RGB565 was also flagged but turns out to be
genuinely platform-conditional (Makefile.common defines it when
WANT_32BPP=0). Kept.
================================================================
F. assert() audit
================================================================
Two assert() calls live in src/ntsc/nes_ntsc_impl.h - third-party
NTSC filter code from blargg, both NaN sanity checks
(`assert(x == x)`). NDEBUG is in the build flags so they compile
to no-ops. No fceumm code uses assert. Nothing to do here.
================================================================
G. const-correctness
================================================================
Function signatures that take strings they don't modify now take
const char *:
FCEU_printf, FCEU_PrintError (const char *format)
FCEUD_PrintError, FCEUD_Message (const char *)
FCEU_MakeFName (const char *cd1)
md5_update (const uint8_t *input)
md5_process (const uint8_t data[64])
also made `static`
================================================================
H. Const-fold static lookup tables
================================================================
Marked static lookup tables const where they are never written:
x6502.c: CycTable[256] (cycle counts)
md5.c: md5_padding[64] (MD5 padding)
vsuni.c: secdata[2][32], secptr (VS security data)
palette.c: rtmul/gtmul/btmul[7] (palette multipliers)
input/cursor.c: GunSight, FCEUcursor (sprite data)
input/pec586kb.c, fkb.c, suborkb.c: matrix (key matrices)
boards/8237.c: regperm, adrperm, protarray (mapper perms)
boards/datalatch.c: M538Banks (bank table)
boards/187.c: prot_data
boards/121.c: prot_array
boards/bonza.c: sim0reset
boards/pec-586.c: bs_tbl, br_tbl
boards/178.c: step_size, step_adj (ADPCM tables)
boards/244.c: prg_perm, chr_perm
boards/bmc42in1r.c: banks
boards/emu2413.c: SL (sustain levels)
NSFROM in nsf.c looks like a lookup table but is rewritten at
runtime to patch in addresses, so it stays mutable.
================================================================
I. Reduce strlen calls
================================================================
Replaced `strlen(STRING_LITERAL)` with `sizeof(STRING_LITERAL) - 1`
where the argument is a compile-time-known string literal:
- libretro.c retro_set_environment APU loop: was calling
strlen("fceumm_apu_") on every loop iteration to compute the
same constant offset.
- nsf.c visualizer: strlen("Song:").
Other strlen sites in cheat.c, libretro.c, unif.c either already
cache to a local size_t or operate on runtime-supplied strings
where caching would not help.
================================================================
Build status
================================================================
`make platform=unix` clean: zero errors, zero warnings.
With -Wsign-compare -Wstrict-aliasing=2 -Wcast-align: zero warnings.
audit_determinism.py: 0 issues.
Output binary 4,388,408 bytes (was 4,388,576 from upstream
004c147; -168 bytes from dead-code removal).
This commit is contained in:
U-DESKTOP-SPFP6AQ\twistedtechre
@@ -346,12 +346,12 @@ static struct retro_log_callback log_cb;
|
||||
|
||||
static void default_logger(enum retro_log_level level, const char *fmt, ...) {}
|
||||
|
||||
void FCEUD_PrintError(char *c)
|
||||
void FCEUD_PrintError(const char *c)
|
||||
{
|
||||
log_cb.log(RETRO_LOG_WARN, "%s", c);
|
||||
}
|
||||
|
||||
void FCEUD_Message(char *s)
|
||||
void FCEUD_Message(const char *s)
|
||||
{
|
||||
log_cb.log(RETRO_LOG_INFO, "%s", s);
|
||||
}
|
||||
@@ -2291,7 +2291,7 @@ static void check_variables(bool startup)
|
||||
|
||||
if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && var.value)
|
||||
{
|
||||
unsigned oldval = aspect_ratio_par;
|
||||
int oldval = aspect_ratio_par;
|
||||
if (!strcmp(var.value, "8:7 PAR")) {
|
||||
aspect_ratio_par = 1;
|
||||
} else if (!strcmp(var.value, "4:3")) {
|
||||
@@ -2443,7 +2443,10 @@ static void check_variables(bool startup)
|
||||
strlcpy(key, "fceumm_apu_x", sizeof(key));
|
||||
for (i = 0; i < 5; i++)
|
||||
{
|
||||
key[strlen("fceumm_apu_")] = '1' + i;
|
||||
/* Replace the trailing 'x' with '1'..'5'. The literal length is
|
||||
* known at compile time so use sizeof-1 to avoid the strlen call
|
||||
* inside the loop. */
|
||||
key[sizeof("fceumm_apu_") - 1] = '1' + i;
|
||||
var.value = NULL;
|
||||
if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && !strcmp(var.value, "disabled"))
|
||||
enable_apu &= ~(1 << i);
|
||||
@@ -2890,7 +2893,7 @@ static void FCEUD_UpdateInput(void)
|
||||
}
|
||||
else /* palette_next */
|
||||
{
|
||||
if (new_palette_index < PAL_TOTAL - 1)
|
||||
if ((unsigned long)new_palette_index < PAL_TOTAL - 1)
|
||||
new_palette_index++;
|
||||
else
|
||||
new_palette_index = 0;
|
||||
@@ -3094,7 +3097,7 @@ bool retro_serialize(void *data, size_t size)
|
||||
if (geniestage == 1)
|
||||
return false;
|
||||
|
||||
if (size != retro_serialize_size())
|
||||
if (!data || size != retro_serialize_size())
|
||||
return false;
|
||||
|
||||
memstream_set_buffer((uint8_t*)data, size);
|
||||
@@ -3109,7 +3112,7 @@ bool retro_unserialize(const void * data, size_t size)
|
||||
if (geniestage == 1)
|
||||
return false;
|
||||
|
||||
if (size != retro_serialize_size())
|
||||
if (!data || size != retro_serialize_size())
|
||||
return false;
|
||||
|
||||
memstream_set_buffer((uint8_t*)data, size);
|
||||
|
||||
Reference in New Issue
Block a user