Read each physical configuration line independently so malformed or truncated input cannot consume a following setting as its value. Discard overlong lines and retain defaults for incomplete entries.
Assisted-by: Codex:gpt-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
blit_screen() branches purely on pinfo.bpp (16 vs 32) to decide
between the RGB565 and RGB32 conversion paths, but bit depth alone
doesn't determine pixel layout - multiple incompatible formats share
the same depth. i_init_graphics() now checks vinfo.fmt against the one
format blit_screen() actually emits for each depth (FB_FMT_RGB16_565
for 16bpp, FB_FMT_RGB32 for 32bpp) and fails loudly via i_error() on
any mismatch or unsupported depth, instead of silently misinterpreting
the framebuffer's actual pixel layout.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
A supervisor process (e.g. an app-store UI that owns the framebuffer
while a launched game runs directly against /dev/fb0) has no reachable
in-game quit path to drive - no keyboard/touch input is wired up for
that - so it can only ask NXDoom to exit from the outside. The only
existing option for that was a forced task_delete() from the
supervisor's side, which was found on real hardware to hang the
entire board (not just this one task) when it landed mid framebuffer/
heap access, on this flat-memory build where a bad access in one task
isn't contained to that task.
Add i_install_quit_signal()/i_poll_quit_signal()/a SIGTERM handler
(i_system.c/i_system.h): the handler itself only sets a volatile
sig_atomic_t flag - it must not call i_quit() (or anything it does:
munmap, fclose, exit()'s atexit chain) directly, since a signal can
land at literally any point in this process's own execution,
including mid-malloc()/mid-blit, the same "unsafe mid-operation
teardown" risk as being force-killed from outside, just moved into
this process's own context. i_poll_quit_signal() defers the actual
shutdown to a call in the main per-frame loop (d_doomloop(), d_main.c)
- a point that's definitely safe (outside any framebuffer/heap access)
and reached every frame regardless of what else NXDoom is doing, which
is what's actually verified working end-to-end against a real
supervisor's SIGTERM/close path on hardware.
i_install_quit_signal() is called once from main() (i_main.c), and
also:
- Checks sigaction()'s return value and logs via syslog on failure
instead of ignoring it silently - the game still runs either way,
but silently leaving close non-functional with no trace of why
would make a real close-path bug harder to diagnose than it needs
to be.
- Resets quit_requested and exit_funcs at the start of
i_install_quit_signal(). This board's flat, single address-space
build normally relaunches NXDoom as a fresh loadable ELF module with
its own zeroed .bss, but GAMES_NXDOOM is a tristate Kconfig symbol
and can also be built in as a true built-in sharing this process's
address space across "launches" with no fresh .bss at all - a
leaked quit_requested flag would call i_quit() again before the game
even starts on a second invocation, and a leaked exit_funcs chain
would re-run every previous invocation's exit handlers a second
time.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
load_default_collection()'s `while (!feof(f))` loop decided whether to
keep looping off feof() rather than fscanf()'s own return value - the
classic version of this bug: on a config file whose bytes don't line
up with "%s %[^\n]\n" at all (e.g. leftover binary/corrupted content
from a previous write), fscanf() can fail a conversion without the
stream ever reaching EOF, and feof() has no way to know that. On real
hardware this hung NXDoom completely at startup with a corrupted
default.cfg on disk - no crash, no output, and no way to close the
game either, since the hang happened before the main loop (and its
SIGTERM poll point) was ever reached.
Terminate directly off EOF/error from fscanf() instead. A failed
conversion is only guaranteed to consume nothing, not to advance the
stream, so also track the file position directly and force one byte
of progress (or bail out) if a scan attempt didn't move it - corrupt/
binary content can now only ever cost one pass over the file, never
an infinite loop.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
r_map_plane()'s existing bounds check clamped an out-of-range `y` to
SCREENHEIGHT - 1, believing that to be the array bound in need of
protection. On real hardware that clamp itself caused a crash: `y` is
stored into ds_y and later used by r_draw_span() (r_draw.c) to index
ylookup[], which r_init_buffer() only populates for [0, viewheight) -
viewheight can be smaller than SCREENHEIGHT (a sub-window within the
physical screen), so entries from viewheight up to SCREENHEIGHT are
zero-initialized (NULL) pointers. Clamping to SCREENHEIGHT - 1 traded
the original out-of-bounds write for a NULL-pointer-plus-offset
framebuffer write, confirmed on real hardware as a load/store
exception at a small virtual address. viewheight is always <=
SCREENHEIGHT, so clamping to viewheight - 1 instead is safe for
cachedheight[]/cacheddistance[]/cachedxstep[]/cachedystep[] too.
r_draw.c's own RANGECHECK-gated debug assertions are updated to match
(they previously compared against SCREENHEIGHT as well).
Separately, r_make_spans() indexes spanstart[t1]/spanstart[b1] (read)
and writes spanstart[t2]/spanstart[b2] using row indices taken
directly from a visplane's top[]/bottom[] arrays, before r_map_plane()
is ever called - so its clamp can't protect these. In valid play these
rows are either a real screen row or vanilla DOOM's 0xff (255) "no
span here" sentinel, and the surrounding while-loop guards are written
so the sentinel can never reach spanstart[] except at a plane's own
edge columns, where writing spanstart[255] is part of the normal
algorithm - already out-of-bounds on this port, since spanstart[] is
only sized SCREENHEIGHT (200). A corrupted BSP/segment can also hand
these a genuinely arbitrary value (this is what the row-255 crash
above traced back to). Guard every touch of spanstart[] directly
instead of altering t1/b1/t2/b2 themselves, so the span-tracking state
machine's comparisons - including the sentinel logic they rely on -
are completely unaffected; a row outside the array just contributes 0
as its span start instead of corrupting or reading past memory.
Also gives GAMES_NXDOOM_STATDUMP_MAX_CAPTURES an explicit range (1
1024) - this diagnostic capture-buffer size Kconfig option had no
bound at all.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Add 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. A single blit loop now
branches on pinfo.bpp only at the pixel-write step (RGBTO16() for
16bpp, the existing ARGBTO32() path for 32bpp); anything else fails
loudly via i_error() rather than 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, and adds
CONFIG_GAMES_NXDOOM_PREFDIR to the IWAD search path (previously only
used for the config/save file location; the Kconfig entry always has a
default, so no #ifdef guard is needed around using it).
Makes GAMES_NXDOOM tristate and lets MODULE follow it
(MODULE = $(CONFIG_GAMES_NXDOOM)) instead of hardcoding MODULE = m, so
it can still be built in as before or selected as a standalone
loadable module installable via nxpkg/nxstore, same as every other
tristate-capable app in apps/.
Fix two real hardware crashes found while bringing this up as a
loadable module:
- A truncated/corrupted config line was silently overriding a
variable's compiled-in default with an empty/unparsable value
instead of being skipped - this let a corrupted "screenblocks" line
through as screenblocks=0, and the renderer's view-size math divides
by a value derived from screenblocks, reaching a divide-by-zero
hardware exception. Also clamps screenblocks to its own valid range
[3, 11] as an independent second layer of defense.
- r_map_plane()'s bounds check was gated behind the debug-only
CONFIG_GAMES_NXDOOM_RANGECHECK and, when tripped, called the fatal
i_error() - both wrong: the check guards a real out-of-bounds array
access (observed 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 rendering the
glitch. Now unconditional and clamps y into range instead of
touching memory outside the buffers' bounds, so the span still
renders (as one glitched row) rather than leaving a gap.
Also adds CONFIG_GAMES_NXDOOM_HEAP_BUFFERS: the renderer's
visplanes/openings/drawsegs/vissprites scratch buffers remain static
arrays by default (matching vanilla DOOM), with heap allocation
available as an opt-in for targets where their combined size threatens
the internal DRAM budget once linked into a full application image.
CONFIG_GAMES_NXDOOM_STATDUMP_MAX_CAPTURES makes statdump's diagnostic
capture-buffer size (unrelated to gameplay) a Kconfig option instead of
a hardcoded value, default unchanged from vanilla (32).
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
NuttX has dirent, so the glob implementation can be used. This removes
the warning for no native glob implementation.
Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
This commit resolves all of the build warnings that were preventing
NXDoom from compiling in CI runs.
Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
This commit includes a (highly) modified version of Chocolate DOOM which
can run on NuttX. The majority of the modifications were made to pass
the NuttX style check. Some small modifications have been added to
support keyboard input, render graphics to frame buffers and directly
use the POSIX interfaces NuttX supplies, stripping out Windows/Mac stuff
and any references to SDL.
NOTE: Sound is currently not supported in any capacity, nor is the
networking stuff. A lot of Chocolate DOOM code was stripped out since it
was unused. If there is a need/desire to add it back later, the original
Chocolate DOOM source can be used as a reference.
WARNING: The NuttX keyboard codec is incredibly non-standard and so
there are problems translating from X11 keys to NuttX ones to play DOOM.
Right now, the CTRL key for firing doesn't work because the NuttX codec
has no concept of it. The NuttX codec should be modified (and other
input devices supported), but at the time of this port I am not
sufficiently comfortable doing so since I am afraid of breaking other
things in the kernel.
NOTE: This port (and likely the original DOOM) seems to be written with
32-bit computers in mind. As such, most things are given the type of
natural `int`, even when a single byte might do. There are significant
size optimizations that could be made to make this more suited to
embedded devices that NuttX typically runs on.
Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
This commit adds the original Chocolate DOOM source which forms a basis
for the NuttX port of DOOM.
Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
This commit introduces a new application, Conway's Game of Life (or
`cgol`). It is a simple frame buffer rendering application that makes
for an interesting, animated visual.
Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Most tools used for compliance and SBOM generation use SPDX identifiers
This change brings us a step closer to an easy SBOM generation.
Signed-off-by: Alin Jerpelea <alin.jerpelea@sony.com>
