aviralgarg05 commented on code in PR #3644:
URL: https://github.com/apache/nuttx-apps/pull/3644#discussion_r3597245520
##########
games/NXDoom/src/i_main.c:
##########
@@ -57,17 +57,28 @@ void d_doom_main(void);
int main(int argc, char **argv)
{
- /* save arguments */
+ /* save arguments
+ *
+ * +1 and an explicit NULL terminator: argv is conventionally
+ * NULL-terminated at argv[argc] (this is what the OS/exec path
+ * guarantees for the `argv` parameter above), and some of this
+ * codebase's own argument handling was written assuming that holds
+ * for myargv too - an under-sized allocation here leaves myargv[argc]
+ * pointing at whatever the allocator happens to return next, which
+ * only reads as "probably zero" by chance depending on heap layout.
+ */
myargc = argc;
- myargv = malloc(argc * sizeof(char *));
+ myargv = malloc((argc + 1) * sizeof(char *));
assert(myargv != NULL);
for (int i = 0; i < argc; i++)
Review Comment:
Yes, a bad-pointer-dereference crash (LoadProhibitedCause) on real hardware,
root-caused through the crash dump and symbol resolution against the built ELF.
myargv was allocated for exactly argc pointers but code elsewhere assumed it
was NULL-terminated at argv[argc], so that slot pointed at whatever the
allocator returned next (usually zero by luck)
##########
games/NXDoom/src/d_iwad.c:
##########
@@ -271,6 +271,18 @@ static void buld_iwad_dir_list(void)
add_iwad_dir(m_dir_name(myargv[0]));
+ /* Add the board's configured DOOM data directory. Kconfig documents
+ * CONFIG_GAMES_NXDOOM_PREFDIR as "Directory where DOOM WAD files are
+ * stored", but until now it was only used for the config/save file
+ * location -- nothing actually searched it for IWADs, forcing every
+ * launch to rely on the current directory or DOOMWADDIR/DOOMWADPATH
+ * being set by hand first.
+ */
+
+#ifdef CONFIG_GAMES_NXDOOM_PREFDIR
+ add_iwad_dir(CONFIG_GAMES_NXDOOM_PREFDIR);
+#endif
Review Comment:
Removing it
##########
games/NXDoom/src/doom/statdump.c:
##########
@@ -39,7 +39,7 @@
* Pre-processor Definitions
****************************************************************************/
-#define MAX_CAPTURES 32
+#define MAX_CAPTURES 4
Review Comment:
will do that
##########
games/NXDoom/src/doom/r_plane.c:
##########
@@ -195,7 +206,48 @@ static void r_make_spans(int x, int t1, int b1, int t2,
int b2)
void r_init_planes(void)
{
- /* Doh! */
+ /* These renderer scratch buffers are sized for a comfortable margin
+ * above vanilla DOOM's original limits and would blow the platform's
+ * internal DRAM budget as static arrays, so they're heap-allocated
+ * instead (comes out of the PSRAM-backed user heap on this target).
+ */
+
+ visplanes = malloc(sizeof(visplane_t) * CONFIG_GAMES_NXDOOM_MAXVISPLANES);
+ openings = malloc(sizeof(short) * MAXOPENINGS);
+ drawsegs = malloc(sizeof(drawseg_t) * CONFIG_GAMES_NXDOOM_MAXDRAWSEGS);
+ vissprites = malloc(sizeof(vissprite_t) *
+ CONFIG_GAMES_NXDOOM_MAXVISSPRITES);
+
+ if (visplanes == NULL || openings == NULL || drawsegs == NULL ||
+ vissprites == NULL)
+ {
+ i_error("r_init_planes: failed to allocate renderer buffers");
+ }
Review Comment:
same as above
--
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]
