aviralgarg05 opened a new pull request, #3644: URL: https://github.com/apache/nuttx-apps/pull/3644
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).* ## Summary This is another slice of the ongoing GSoC 2026 Dynamic ELF loading and `nxpkg` work for NuttX. An earlier draft PR (#3474) carries the initial `nxpkg` package lifecycle helper. This PR is independent of the `nxpkg`/`nxstore` work itself (#3642, #3643) — it's a set of bring-up and hardening fixes to `games/NXDoom` found while porting it to run as one of the packages those PRs' frontend can install and launch. Series order for this PR: - base series context: #3474 (merged) - independent of: #3519, #3526, #3531, #3642, #3643 Concretely, it: - adds RGB565 framebuffer support to `blit_screen()` — it previously assumed a 32-bit ARGB framebuffer unconditionally, a real limitation for any board whose framebuffer is `FB_FMT_RGB16_565`. Branches on `pinfo.bpp`: 16bpp uses `RGBTO16()`, 32bpp keeps the existing `ARGBTO32()` path, anything else fails loudly via `i_error()` instead of reading/writing past the intended pixel bounds silently. Also centers the scaled viewport within the framebuffer instead of pinning it to the top-left corner - adds `CONFIG_GAMES_NXDOOM_PREFDIR` to the IWAD search path — that Kconfig option is documented as where DOOM WAD files are stored, but was previously only used for the config/save file location - builds NXDoom as a standalone loadable module (`MODULE = m`) unconditionally, so it can be installed/launched via `nxpkg`/`nxstore` rather than only ever being a firmware built-in - hardens config-file parsing: a truncated/corrupted config line (e.g. left mid-write by an unclean shutdown) was silently overriding a variable's compiled-in default with an empty/unparsable value instead of being skipped — this is what let a corrupted `screenblocks` line through as `screenblocks=0`, and the renderer's view-size math divides by a value derived from `screenblocks`, so that reached a divide-by-zero hardware exception. Now clamps `screenblocks` to its own valid range `[3, 11]` as an independent second layer of defense - fixes a real out-of-bounds array write in `r_map_plane()`: its bounds check was gated behind the debug-only `CONFIG_GAMES_NXDOOM_RANGECHECK` and, when tripped, called the fatal `i_error()`. Both were wrong — the check guards a real out-of-bounds write (observed on real hardware with values far past even `viewheight`), so it cannot be optional; and killing the whole process over one glitched plane span is worse than vanilla DOOM's own behavior of just rendering the glitch. The check is now unconditional, checks against the true array bound (`SCREENHEIGHT`, not `viewheight`), and skips the draw instead of touching memory or aborting - converts `visplanes`/`openings`/`drawsegs`/`vissprites` from static arrays to heap-allocated buffers (allocated once, freed via `i_at_exit(..., true)` so cleanup also runs on the `i_error()` fatal path) — these buffers are large enough as static arrays to threaten this kind of target's internal DRAM budget once a full application is linked into the same image; this target's user heap is PSRAM-backed, so heap allocation removes that pressure. Also reduces `statdump.c`'s `MAX_CAPTURES` from 32 to 4 for the same reason (this buffer is diagnostic capture storage, not required for gameplay) - fixes an under-allocated `argv` array: `myargv` was sized for `argc` pointers, but this codebase's own argument-handling assumes `argv` is `NULL`-terminated at `argv[argc]` (standard C/exec convention). The under-sized allocation left `myargv[argc]` pointing at whatever the allocator happened to return next, which only read as `NULL` by chance depending on heap layout — root-caused via a real crash dump and symbol resolution against the built ELF ## Impact User impact: - NXDoom now renders correctly on RGB565 framebuffers, not just 32-bit ARGB ones - NXDoom is now installable/launchable as a standalone package instead of only ever being compiled into the firmware image - fixes multiple real hardware crashes (divide-by-zero, out-of-bounds write, bad pointer dereference) Build impact: - `games/NXDoom`'s `MODULE` is now unconditionally `m` Runtime impact: - `visplanes`/`openings`/`drawsegs`/`vissprites` move from static to heap allocation, changing this app's memory footprint profile (lower static/BSS usage, one-time heap allocation at startup instead) Compatibility impact: - none for other applications; this PR only touches `games/NXDoom` ## Testing Host used for build: - OS: macOS 26.5 arm64 - compiler: `xtensa-esp-elf-gcc (crosstool-NG esp-14.2.0_20241119) 14.2.0` Target used for verification: - arch: `xtensa` - board/config used during runtime verification: `esp32s3-touch-lcd-7:nsh` - board: Waveshare ESP32-S3-Touch-LCD-7 (custom board directory, not yet upstream) Runtime flow verified on hardware, repeatedly, across this project's full install/launch/close lifecycle: 1. boot to the touchscreen app list (see #3643) 2. install and launch NXDoom as a standalone package 3. renders correctly on this board's RGB565 framebuffer, to the title screen and into gameplay 4. play for an extended session without hitting the divide-by-zero, out-of-bounds-write, or bad-`argv`-pointer crashes this PR fixes — each of those was found and root-caused via an actual crash on this exact board before being fixed here 5. close cleanly and return to the app list (the cooperative-quit side of that handshake is a separate PR) This is a draft PR — opening now to get the series in front of reviewers; happy to split further if any single commit above should be its own PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
