Skip to content

Improve platform code robustness and fix bugs. #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nchapman merged 2 commits into from
Dec 8, 2025
Merged

Improve platform code robustness and fix bugs. #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a353770
Improve platform code robustness and fix bugs.
nchapman Dec 7, 2025
043250a
Review fixes.
nchapman Dec 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commits.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
35 changes: 35 additions & 0 deletions workspace/all/common/platform_variant.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@
*/

#include "platform_variant.h"
#include "platform.h"
#include <stdio.h>

// 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,
Expand All @@ -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];

Expand Down
4 changes: 3 additions & 1 deletion workspace/m17/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
/**
Expand Down
9 changes: 8 additions & 1 deletion workspace/magicmini/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

/**
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions workspace/miyoomini/keymon/keymon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
8 changes: 6 additions & 2 deletions workspace/miyoomini/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
10 changes: 8 additions & 2 deletions workspace/my282/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
30 changes: 27 additions & 3 deletions workspace/rg35xx/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ 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");
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource leak: When mmap fails, info->fd (created by ION_IOC_MAP ioctl at line 78-79) should be closed before returning, as it won't be used.

Add cleanup:

if (info->vadd == MAP_FAILED) {
    fprintf(stderr, "ION mmap failed\n");
    close(info->fd);
    info->vadd = NULL;
}
Suggested change
fprintf(stderr, "ION mmap failed\n");
fprintf(stderr, "ION mmap failed\n");
close(info->fd);

Copilot uses AI. Check for mistakes.
close(info->fd);
info->vadd = NULL;
}
}

static void ion_free(int fd_ion, ion_alloc_info_t* info) {
Expand Down Expand Up @@ -339,12 +344,31 @@ 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);
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource leak: When file descriptor opening fails after some files have been successfully opened, the successfully opened file descriptors should be closed before returning NULL.

Add cleanup code:

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;
}
Suggested change
vid.fd_mem);
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);

Copilot uses AI. Check for mistakes.
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");
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource leak: When mmap fails, the successfully opened file descriptors (vid.fd_fb, vid.fd_ion, vid.fd_mem) should be closed before returning NULL.

Add cleanup:

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;
}
Suggested change
LOG_error("Failed to mmap display engine\n");
LOG_error("Failed to mmap display engine\n");
close(vid.fd_mem);
close(vid.fd_ion);
close(vid.fd_fb);

Copilot uses AI. Check for mistakes.
close(vid.fd_mem);
close(vid.fd_ion);
close(vid.fd_fb);
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)
Expand Down Expand Up @@ -746,7 +770,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);
}

Expand All @@ -771,7 +795,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;
}
Expand Down
2 changes: 2 additions & 0 deletions workspace/rg35xxplus/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 13 additions & 5 deletions workspace/rgb30/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion workspace/tg5040/libmsettings/msettings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,12 @@ 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, &param);
close(fd);
} else {
fprintf(stderr, "SetRawBrightness: failed to open /dev/disp\n");
}
}
void SetRawVolume(int val) { // 0-100
Expand Down
38 changes: 38 additions & 0 deletions workspace/trimuismart/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ 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");
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource leak: When mmap fails, info->fd (created by ION_IOC_MAP ioctl at line 103-104) should be closed before returning, as it won't be used.

Add cleanup:

if (info->vadd == MAP_FAILED) {
    fprintf(stderr, "ION mmap failed\n");
    close(info->fd);
    info->vadd = NULL;
    return;
}
Suggested change
fprintf(stderr, "ION mmap failed\n");
fprintf(stderr, "ION mmap failed\n");
close(info->fd);

Copilot uses AI. Check for mistakes.
close(info->fd);
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);
}
Expand Down Expand Up @@ -201,9 +207,30 @@ 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) {
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) {
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;
}

memset(&vid.fb_config, 0, sizeof(disp_layer_config));
memset(&vid.buffer_config, 0, sizeof(disp_layer_config));
Expand Down Expand Up @@ -509,8 +536,19 @@ void ADC_init(void) {
int addr_offset = LRADC & ~page_mask;

adc.mem_fd = open("/dev/mem", O_RDWR);
if (adc.mem_fd < 0) {
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) {
LOG_error("Failed to mmap ADC registers\n");
close(adc.mem_fd);
return;
}

adc.adc_addr = adc.mem_map + addr_offset;
*(uint32_t*)adc.adc_addr = 0xC0004D;
}
Expand Down
2 changes: 2 additions & 0 deletions workspace/zero28/platform/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down