From a353770f17217dff179859cc8ada0250f3f4d09c Mon Sep 17 00:00:00 2001 From: Nick Chapman Date: Sun, 7 Dec 2025 15:19:08 -0800 Subject: [PATCH 1/2] Improve platform code robustness and fix bugs. - Add fallback PLAT_detectVariant() for single-device platforms - Fix rgb30 HDMI disabled despite hardware support - Fix rg35xxplus crash when RGXX_MODEL env var unset - Add error handling for I2C, mmap, and input device operations - Replace unsafe sprintf/strcpy with bounds-checked variants - Initialize ioctl structures to prevent kernel garbage - Remove debug logging that spams during gameplay --- workspace/all/common/platform_variant.c | 35 +++++++++++++++++++++++ workspace/m17/platform/platform.c | 4 ++- workspace/magicmini/platform/platform.c | 9 +++++- workspace/miyoomini/keymon/keymon.c | 4 +++ workspace/miyoomini/platform/platform.c | 8 ++++-- workspace/my282/platform/platform.c | 10 +++++-- workspace/rg35xx/platform/platform.c | 20 +++++++++++-- workspace/rg35xxplus/platform/platform.c | 2 ++ workspace/rgb30/platform/platform.c | 18 ++++++++---- workspace/tg5040/libmsettings/msettings.c | 2 +- workspace/trimuismart/platform/platform.c | 24 ++++++++++++++++ workspace/zero28/platform/platform.c | 2 ++ 12 files changed, 123 insertions(+), 15 deletions(-) diff --git a/workspace/all/common/platform_variant.c b/workspace/all/common/platform_variant.c index 88881580..a337b318 100644 --- a/workspace/all/common/platform_variant.c +++ b/workspace/all/common/platform_variant.c @@ -3,8 +3,14 @@ */ #include "platform_variant.h" +#include "platform.h" #include +// Weak symbol - platform can override with its own implementation +#ifndef FALLBACK_IMPLEMENTATION +#define FALLBACK_IMPLEMENTATION __attribute__((weak)) +#endif + // Global platform variant instance PlatformVariant platform_variant = {.platform = NULL, .variant = VARIANT_NONE, @@ -17,6 +23,35 @@ PlatformVariant platform_variant = {.platform = NULL, .hw_features = 0, .platform_data = NULL}; +/** + * Default implementation for single-device platforms. + * + * Platforms with multiple device variants should override this with their own + * implementation that includes device detection and registry lookup. + * + * This fallback uses compile-time constants from platform.h. + */ +FALLBACK_IMPLEMENTATION +void PLAT_detectVariant(PlatformVariant* v) { + v->platform = PLATFORM; + v->variant = VARIANT_STANDARD; + v->device = NULL; // Single-device platforms don't need registry + v->screen_width = FIXED_WIDTH; + v->screen_height = FIXED_HEIGHT; + v->screen_diagonal = SCREEN_DIAGONAL; +#ifdef HAS_HDMI + v->has_hdmi = HAS_HDMI; +#else + v->has_hdmi = 0; +#endif + v->hdmi_active = 0; + v->hw_features = 0; +#ifdef HAS_NEON + v->hw_features |= HW_FEATURE_NEON; +#endif + v->platform_data = NULL; +} + const char* PLAT_getDeviceName(void) { static char device_name[256]; diff --git a/workspace/m17/platform/platform.c b/workspace/m17/platform/platform.c index 0aa423bb..ee6f01af 100644 --- a/workspace/m17/platform/platform.c +++ b/workspace/m17/platform/platform.c @@ -74,8 +74,10 @@ static int inputs[INPUT_COUNT]; void PLAT_initInput(void) { char path[256]; for (int i = 0; i < INPUT_COUNT; i++) { - sprintf(path, "/dev/input/event%i", i); + snprintf(path, sizeof(path), "/dev/input/event%i", i); inputs[i] = open(path, O_RDONLY | O_NONBLOCK | O_CLOEXEC); + if (inputs[i] < 0) + LOG_warn("Failed to open /dev/input/event%d\n", i); } } /** diff --git a/workspace/magicmini/platform/platform.c b/workspace/magicmini/platform/platform.c index 2982082c..dcf77997 100644 --- a/workspace/magicmini/platform/platform.c +++ b/workspace/magicmini/platform/platform.c @@ -101,6 +101,13 @@ void PLAT_initInput(void) { inputs[0] = open("/dev/input/event0", O_RDONLY | O_NONBLOCK | O_CLOEXEC); // power inputs[1] = open("/dev/input/event2", O_RDONLY | O_NONBLOCK | O_CLOEXEC); // gamepad inputs[2] = open("/dev/input/event3", O_RDONLY | O_NONBLOCK | O_CLOEXEC); // volume + + if (inputs[0] < 0) + LOG_warn("Failed to open power input (event0)\n"); + if (inputs[1] < 0) + LOG_warn("Failed to open gamepad input (event2)\n"); + if (inputs[2] < 0) + LOG_warn("Failed to open volume input (event3)\n"); } /** @@ -215,7 +222,7 @@ void PLAT_pollInput(void) { } } else if (type == EV_ABS) { // Analog stick events - left stick generates digital button presses - LOG_info("abs event: %i (%i)\n", code, value); + // LOG_info("abs event: %i (%i)\n", code, value); if (code == RAW_LSX) { pad.laxis.x = value; PAD_setAnalog(BTN_ID_ANALOG_LEFT, BTN_ID_ANALOG_RIGHT, pad.laxis.x, diff --git a/workspace/miyoomini/keymon/keymon.c b/workspace/miyoomini/keymon/keymon.c index a7a73ecb..16eefeb2 100644 --- a/workspace/miyoomini/keymon/keymon.c +++ b/workspace/miyoomini/keymon/keymon.c @@ -108,6 +108,8 @@ int axp_write(unsigned char address, unsigned char val) { unsigned char buf[2]; int ret; int fd = open(AXPDEV, O_RDWR); + if (fd < 0) + return -1; ioctl(fd, I2C_TIMEOUT, 5); ioctl(fd, I2C_RETRIES, 1); @@ -152,6 +154,8 @@ int axp_read(unsigned char address) { unsigned char val; int ret; int fd = open(AXPDEV, O_RDWR); + if (fd < 0) + return -1; ioctl(fd, I2C_TIMEOUT, 5); ioctl(fd, I2C_RETRIES, 1); diff --git a/workspace/miyoomini/platform/platform.c b/workspace/miyoomini/platform/platform.c index c6ee1d8b..d47eeda5 100644 --- a/workspace/miyoomini/platform/platform.c +++ b/workspace/miyoomini/platform/platform.c @@ -715,6 +715,8 @@ int axp_write(unsigned char address, unsigned char val) { unsigned char buf[2]; int ret; int fd = open(AXPDEV, O_RDWR); + if (fd < 0) + return -1; ioctl(fd, I2C_TIMEOUT, 5); ioctl(fd, I2C_RETRIES, 1); @@ -741,6 +743,8 @@ int axp_read(unsigned char address) { unsigned char val; int ret; int fd = open(AXPDEV, O_RDWR); + if (fd < 0) + return -1; ioctl(fd, I2C_TIMEOUT, 5); ioctl(fd, I2C_RETRIES, 1); @@ -851,7 +855,7 @@ void PLAT_setCPUSpeed(int speed) { LOG_info("PLAT_setCPUSpeed: %s (%d kHz)\n", level_name, freq); char cmd[32]; - sprintf(cmd, "overclock.elf %d\n", freq); + snprintf(cmd, sizeof(cmd), "overclock.elf %d", freq); int ret = system(cmd); if (ret != 0) { LOG_warn("overclock.elf returned %d for freq %d\n", ret, freq); @@ -879,7 +883,7 @@ int PLAT_getAvailableCPUFrequencies(int* frequencies, int max_count) { */ int PLAT_setCPUFrequency(int freq_khz) { char cmd[32]; - sprintf(cmd, "overclock.elf %d\n", freq_khz); + snprintf(cmd, sizeof(cmd), "overclock.elf %d", freq_khz); int ret = system(cmd); return (ret == 0) ? 0 : -1; } diff --git a/workspace/my282/platform/platform.c b/workspace/my282/platform/platform.c index f0092a19..6339ef71 100644 --- a/workspace/my282/platform/platform.c +++ b/workspace/my282/platform/platform.c @@ -74,6 +74,12 @@ static int inputs[INPUT_COUNT]; void PLAT_initInput(void) { inputs[0] = open("/dev/input/event0", O_RDONLY | O_NONBLOCK | O_CLOEXEC); // power inputs[1] = open("/dev/input/event3", O_RDONLY | O_NONBLOCK | O_CLOEXEC); // controller + + if (inputs[0] < 0) + LOG_warn("Failed to open power input (event0)\n"); + if (inputs[1] < 0) + LOG_warn("Failed to open controller input (event3)\n"); + Stick_init(); // analog } @@ -392,7 +398,7 @@ void PLAT_setCPUSpeed(int speed) { char cmd[128]; // Set CPU governor to userspace mode with specified cores and frequency - sprintf(cmd, "overclock.elf userspace %d %d 384 1080 0", cpu, freq); + snprintf(cmd, sizeof(cmd), "overclock.elf userspace %d %d 384 1080 0", cpu, freq); system(cmd); } @@ -421,7 +427,7 @@ int PLAT_getAvailableCPUFrequencies(int* frequencies, int max_count) { int PLAT_setCPUFrequency(int freq_khz) { int freq_mhz = freq_khz / 1000; char cmd[128]; - sprintf(cmd, "overclock.elf userspace 2 %d 384 1080 0", freq_mhz); + snprintf(cmd, sizeof(cmd), "overclock.elf userspace 2 %d 384 1080 0", freq_mhz); int ret = system(cmd); return (ret == 0) ? 0 : -1; } diff --git a/workspace/rg35xx/platform/platform.c b/workspace/rg35xx/platform/platform.c index 21e9be8d..423a4ed7 100644 --- a/workspace/rg35xx/platform/platform.c +++ b/workspace/rg35xx/platform/platform.c @@ -82,6 +82,10 @@ static void ion_alloc(int fd_ion, ion_alloc_info_t* info) { info->fd = ifd.fd; info->padd = (void*)ipd.phys_addr; info->vadd = mmap(0, info->size, PROT_READ | PROT_WRITE, MAP_SHARED, info->fd, 0); + if (info->vadd == MAP_FAILED) { + fprintf(stderr, "ION mmap failed\n"); + info->vadd = NULL; + } } static void ion_free(int fd_ion, ion_alloc_info_t* info) { @@ -339,12 +343,22 @@ SDL_Surface* PLAT_initVideo(void) { vid.fd_fb = open("/dev/fb0", O_RDWR); vid.fd_ion = open("/dev/ion", O_RDWR); vid.fd_mem = open("/dev/mem", O_RDWR); + if (vid.fd_fb < 0 || vid.fd_ion < 0 || vid.fd_mem < 0) { + LOG_error("Failed to open device files (fb=%d ion=%d mem=%d)\n", vid.fd_fb, vid.fd_ion, + vid.fd_mem); + return NULL; + } + vid.de_mem = mmap(0, DE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, vid.fd_mem, DE); + if (vid.de_mem == MAP_FAILED) { + LOG_error("Failed to mmap display engine\n"); + return NULL; + } ioctl(vid.fd_fb, FBIOGET_FSCREENINFO, &vid.finfo); ioctl(vid.fd_fb, FBIOGET_VSCREENINFO, &vid.vinfo); - struct owlfb_sync_info sinfo; + struct owlfb_sync_info sinfo = {0}; sinfo.enabled = 1; sinfo.disp_id = 2; if (ioctl(vid.fd_fb, OWLFB_VSYNC_EVENT_EN, &sinfo) < 0) @@ -746,7 +760,7 @@ void PLAT_setCPUSpeed(int speed) { } char cmd[32]; - sprintf(cmd, "overclock.elf %d\n", freq); + snprintf(cmd, sizeof(cmd), "overclock.elf %d", freq); system(cmd); } @@ -771,7 +785,7 @@ int PLAT_getAvailableCPUFrequencies(int* frequencies, int max_count) { */ int PLAT_setCPUFrequency(int freq_khz) { char cmd[32]; - sprintf(cmd, "overclock.elf %d\n", freq_khz); + snprintf(cmd, sizeof(cmd), "overclock.elf %d", freq_khz); int ret = system(cmd); return (ret == 0) ? 0 : -1; } diff --git a/workspace/rg35xxplus/platform/platform.c b/workspace/rg35xxplus/platform/platform.c index 612ebc87..20a6089b 100644 --- a/workspace/rg35xxplus/platform/platform.c +++ b/workspace/rg35xxplus/platform/platform.c @@ -137,6 +137,8 @@ void PLAT_detectVariant(PlatformVariant* v) { // Read model string from environment char* model = getenv("RGXX_MODEL"); + if (!model) + model = "RG35xxPlus"; // Fallback to default // Look up device in mapping table const DeviceVariantMap* map = NULL; diff --git a/workspace/rgb30/platform/platform.c b/workspace/rgb30/platform/platform.c index cd307460..4adb4bc8 100644 --- a/workspace/rgb30/platform/platform.c +++ b/workspace/rgb30/platform/platform.c @@ -48,7 +48,7 @@ static const SDL2_Config vid_config = { .rotate_cw = 0, .rotate_null_center = 0, // Display features - .has_hdmi = 0, + .has_hdmi = 1, .default_sharpness = SHARPNESS_SOFT, }; @@ -149,6 +149,11 @@ void PLAT_initInput(void) { inputs[1] = open("/dev/input/event1", O_RDONLY | O_NONBLOCK | O_CLOEXEC); inputs[2] = open("/dev/input/event2", O_RDONLY | O_NONBLOCK | O_CLOEXEC); inputs[3] = open("/dev/input/event3", O_RDONLY | O_NONBLOCK | O_CLOEXEC); + + for (int i = 0; i < INPUT_COUNT; i++) { + if (inputs[i] < 0) + LOG_warn("Failed to open /dev/input/event%d\n", i); + } } void PLAT_quitInput(void) { @@ -371,10 +376,13 @@ char* PLAT_getModel(void) { char buffer[256]; getFile("/proc/device-tree/model", buffer, 256); char* tmp = strrchr(buffer, ' '); - if (tmp) - strcpy(model, tmp + 1); - else - strcpy(model, "RGB30"); + if (tmp) { + strncpy(model, tmp + 1, sizeof(model) - 1); + model[sizeof(model) - 1] = '\0'; + } else { + strncpy(model, "RGB30", sizeof(model) - 1); + model[sizeof(model) - 1] = '\0'; + } return model; } diff --git a/workspace/tg5040/libmsettings/msettings.c b/workspace/tg5040/libmsettings/msettings.c index 9c1b75f3..4e350f88 100644 --- a/workspace/tg5040/libmsettings/msettings.c +++ b/workspace/tg5040/libmsettings/msettings.c @@ -188,7 +188,7 @@ void SetRawBrightness(int val) { // 0 - 255 printf("SetRawBrightness(%i)\n", val); fflush(stdout); int fd = open("/dev/disp", O_RDWR); - if (fd) { + if (fd >= 0) { unsigned long param[4]={0,val,0,0}; ioctl(fd, DISP_LCD_SET_BRIGHTNESS, ¶m); close(fd); diff --git a/workspace/trimuismart/platform/platform.c b/workspace/trimuismart/platform/platform.c index 768d1323..c31bfc47 100644 --- a/workspace/trimuismart/platform/platform.c +++ b/workspace/trimuismart/platform/platform.c @@ -107,6 +107,11 @@ void ion_alloc(int ion_fd, ion_alloc_info_t* info) { info->fd = ifd.fd; info->padd = (void*)spd.phys_addr; info->vadd = mmap(0, info->size, PROT_READ | PROT_WRITE, MAP_SHARED, info->fd, 0); + if (info->vadd == MAP_FAILED) { + fprintf(stderr, "ION mmap failed\n"); + info->vadd = NULL; + return; + } fprintf(stderr, "allocated padd: 0x%x vadd: 0x%x size: 0x%x\n", (uintptr_t)info->padd, (uintptr_t)info->vadd, info->size); } @@ -201,9 +206,18 @@ SDL_Surface* PLAT_initVideo(void) { vid.fb_fd = open("/dev/fb0", O_RDWR); vid.ion_fd = open("/dev/ion", O_RDWR); vid.mem_fd = open("/dev/mem", O_RDWR); + if (vid.disp_fd < 0 || vid.fb_fd < 0 || vid.ion_fd < 0 || vid.mem_fd < 0) { + fprintf(stderr, "Failed to open device files (disp=%d fb=%d ion=%d mem=%d)\n", vid.disp_fd, + vid.fb_fd, vid.ion_fd, vid.mem_fd); + return NULL; + } vid.mem_map = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE, MAP_SHARED, vid.mem_fd, OVL_V); + if (vid.mem_map == MAP_FAILED) { + fprintf(stderr, "Failed to mmap overlay registers\n"); + return NULL; + } memset(&vid.fb_config, 0, sizeof(disp_layer_config)); memset(&vid.buffer_config, 0, sizeof(disp_layer_config)); @@ -509,8 +523,18 @@ void ADC_init(void) { int addr_offset = LRADC & ~page_mask; adc.mem_fd = open("/dev/mem", O_RDWR); + if (adc.mem_fd < 0) { + fprintf(stderr, "Failed to open /dev/mem for ADC\n"); + return; + } + adc.mem_map = mmap(0, adc.page_size * 2, PROT_READ | PROT_WRITE, MAP_SHARED, adc.mem_fd, addr_start); + if (adc.mem_map == MAP_FAILED) { + fprintf(stderr, "Failed to mmap ADC registers\n"); + return; + } + adc.adc_addr = adc.mem_map + addr_offset; *(uint32_t*)adc.adc_addr = 0xC0004D; } diff --git a/workspace/zero28/platform/platform.c b/workspace/zero28/platform/platform.c index 9273929d..641bf7eb 100644 --- a/workspace/zero28/platform/platform.c +++ b/workspace/zero28/platform/platform.c @@ -54,6 +54,8 @@ static SDL_Joystick* joystick; void PLAT_initInput(void) { SDL_InitSubSystem(SDL_INIT_JOYSTICK); joystick = SDL_JoystickOpen(0); + if (!joystick) + LOG_warn("Failed to open joystick: %s\n", SDL_GetError()); } /** From 043250a779658ebc6c569debab938d4b07dabedc Mon Sep 17 00:00:00 2001 From: Nick Chapman Date: Sun, 7 Dec 2025 16:01:50 -0800 Subject: [PATCH 2/2] Review fixes. --- commits.sh | 2 +- workspace/rg35xx/platform/platform.c | 10 ++++++++++ workspace/tg5040/libmsettings/msettings.c | 2 ++ workspace/trimuismart/platform/platform.c | 24 ++++++++++++++++++----- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/commits.sh b/commits.sh index 9f1eb6f8..ea721b5d 100755 --- a/commits.sh +++ b/commits.sh @@ -229,4 +229,4 @@ count_toolchains() { echo " - Libretro cores: $TOTAL_CORES" echo " - Other dependencies: $((TOTAL_DEPS - TOTAL_TOOLCHAINS - TOTAL_CORES - 1))" bump -} | sed 's/\n/ /g' +} diff --git a/workspace/rg35xx/platform/platform.c b/workspace/rg35xx/platform/platform.c index 423a4ed7..ed269dd7 100644 --- a/workspace/rg35xx/platform/platform.c +++ b/workspace/rg35xx/platform/platform.c @@ -84,6 +84,7 @@ static void ion_alloc(int fd_ion, ion_alloc_info_t* info) { info->vadd = mmap(0, info->size, PROT_READ | PROT_WRITE, MAP_SHARED, info->fd, 0); if (info->vadd == MAP_FAILED) { fprintf(stderr, "ION mmap failed\n"); + close(info->fd); info->vadd = NULL; } } @@ -346,12 +347,21 @@ SDL_Surface* PLAT_initVideo(void) { if (vid.fd_fb < 0 || vid.fd_ion < 0 || vid.fd_mem < 0) { LOG_error("Failed to open device files (fb=%d ion=%d mem=%d)\n", vid.fd_fb, vid.fd_ion, vid.fd_mem); + if (vid.fd_fb >= 0) + close(vid.fd_fb); + if (vid.fd_ion >= 0) + close(vid.fd_ion); + if (vid.fd_mem >= 0) + close(vid.fd_mem); return NULL; } vid.de_mem = mmap(0, DE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, vid.fd_mem, DE); if (vid.de_mem == MAP_FAILED) { LOG_error("Failed to mmap display engine\n"); + close(vid.fd_mem); + close(vid.fd_ion); + close(vid.fd_fb); return NULL; } diff --git a/workspace/tg5040/libmsettings/msettings.c b/workspace/tg5040/libmsettings/msettings.c index 4e350f88..8cc0bd99 100644 --- a/workspace/tg5040/libmsettings/msettings.c +++ b/workspace/tg5040/libmsettings/msettings.c @@ -192,6 +192,8 @@ void SetRawBrightness(int val) { // 0 - 255 unsigned long param[4]={0,val,0,0}; ioctl(fd, DISP_LCD_SET_BRIGHTNESS, ¶m); close(fd); + } else { + fprintf(stderr, "SetRawBrightness: failed to open /dev/disp\n"); } } void SetRawVolume(int val) { // 0-100 diff --git a/workspace/trimuismart/platform/platform.c b/workspace/trimuismart/platform/platform.c index c31bfc47..d302921b 100644 --- a/workspace/trimuismart/platform/platform.c +++ b/workspace/trimuismart/platform/platform.c @@ -109,6 +109,7 @@ void ion_alloc(int ion_fd, ion_alloc_info_t* info) { info->vadd = mmap(0, info->size, PROT_READ | PROT_WRITE, MAP_SHARED, info->fd, 0); if (info->vadd == MAP_FAILED) { fprintf(stderr, "ION mmap failed\n"); + close(info->fd); info->vadd = NULL; return; } @@ -207,15 +208,27 @@ SDL_Surface* PLAT_initVideo(void) { vid.ion_fd = open("/dev/ion", O_RDWR); vid.mem_fd = open("/dev/mem", O_RDWR); if (vid.disp_fd < 0 || vid.fb_fd < 0 || vid.ion_fd < 0 || vid.mem_fd < 0) { - fprintf(stderr, "Failed to open device files (disp=%d fb=%d ion=%d mem=%d)\n", vid.disp_fd, - vid.fb_fd, vid.ion_fd, vid.mem_fd); + LOG_error("Failed to open device files (disp=%d fb=%d ion=%d mem=%d)\n", vid.disp_fd, + vid.fb_fd, vid.ion_fd, vid.mem_fd); + if (vid.disp_fd >= 0) + close(vid.disp_fd); + if (vid.fb_fd >= 0) + close(vid.fb_fd); + if (vid.ion_fd >= 0) + close(vid.ion_fd); + if (vid.mem_fd >= 0) + close(vid.mem_fd); return NULL; } vid.mem_map = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE, MAP_SHARED, vid.mem_fd, OVL_V); if (vid.mem_map == MAP_FAILED) { - fprintf(stderr, "Failed to mmap overlay registers\n"); + LOG_error("Failed to mmap overlay registers\n"); + close(vid.mem_fd); + close(vid.ion_fd); + close(vid.fb_fd); + close(vid.disp_fd); return NULL; } @@ -524,14 +537,15 @@ void ADC_init(void) { adc.mem_fd = open("/dev/mem", O_RDWR); if (adc.mem_fd < 0) { - fprintf(stderr, "Failed to open /dev/mem for ADC\n"); + LOG_error("Failed to open /dev/mem for ADC\n"); return; } adc.mem_map = mmap(0, adc.page_size * 2, PROT_READ | PROT_WRITE, MAP_SHARED, adc.mem_fd, addr_start); if (adc.mem_map == MAP_FAILED) { - fprintf(stderr, "Failed to mmap ADC registers\n"); + LOG_error("Failed to mmap ADC registers\n"); + close(adc.mem_fd); return; }