Skip to content

console: fix writing out of bounds and writing unused memory#229

Merged
DacoTaco merged 9 commits into
devkitPro:masterfrom
LiquidFenrir:console-fixes
Sep 3, 2025
Merged

console: fix writing out of bounds and writing unused memory#229
DacoTaco merged 9 commits into
devkitPro:masterfrom
LiquidFenrir:console-fixes

Conversation

@LiquidFenrir
Copy link
Copy Markdown
Contributor

Adds precision to the expected values of various arguments/results of the console API
New internal helper functions for commonly repeated (tedious and prone to mistakes) pattern
More comments for non-intuitive/non-obvious implementation decisions that had to be done due to the mixing of "units"

NOT TESTED ON HARDWARE (or in any use), but logically sound (more than the previous mixed results, at least)

introduces helper functions to avoid the repetition and mistakes
logic was verified equivalent or better by hand but not runtime-tested
fixes various issues having to do with out of bounds writes
misc changes to extraneous whitespace noticed during the refactor
@Zarithya
Copy link
Copy Markdown
Contributor

Oh hell yeah this fixes the problems I've been having with my app! Thank you!!

@LiquidFenrir
Copy link
Copy Markdown
Contributor Author

LiquidFenrir commented Aug 31, 2025

For the record:
the actual bugs fixed are

- while( cur_row++ < (con->windowY + con->windowHeight) )
+ while( cur_row++ < con->windowHeight )

in __console_clear_from_cursor, that caused an out of bounds write past the end of the allowed console buffer region (and possibly into memory not belonging to a framebuffer at all, causing corruption beyond graphical)
and

- while( cur_row-- )
+ while( --cur_row )

in __console_clear_to_cursor, which was the same but before the start of the buffer.
Everything else is formatting, documentation, and cleanup to avoid repetition and unify.

TODO: during testing and comparison, @DacoTaco noticed a graphical issue in certain cases, with passing xres/conWidth and yres/conHeight to CON_Init/CON_InitEx that's not a multiple of FONT_XSIZE and FONT_YSIZE respectively.
This causes the extra right and bottom pixels not belonging to a full character/tile to behave unreliably (and unexpectedly) between CON_Init and CON_InitEx:

  • these edges are not drawn over at all when using CON_Init, keeping the pixel colors present before and making the console look smaller than defined by the init call
  • they forever hold the previous values of what was in memory in the _console_buffer region for CON_InitEx (most likely fully black/nul bytes), which is copied by the VI callback to the screen framebuffer, giving it the right size but a random static "border".

This was already the case before this PR, but isn't addressed yet, no decision reached on how to handle it (truncating the intended console dimensions in the init to at least have parity between both, maybe forcing the extra pixels to a certain color, or resetting them when fully clearing the console, or other ideas)

@DacoTaco DacoTaco changed the title Further fixes for console out of bounds console: Further fixes for console writing out of bounds Aug 31, 2025
avoids an unwritten right/bottom edge that's not part of a complete tile
this caused graphical issues and inconsistencies between Init and InitEx
document in the function doxygen comment
make values in __console_vipostcb only computed once
ptr directly from the global equal to destbuffer, less indirection
@LiquidFenrir
Copy link
Copy Markdown
Contributor Author

LiquidFenrir commented Sep 3, 2025

Update: decision reached, implemented and documented in the latest commit, is to force con_xres/con_yres to be multiples of font size inside CON_Init/Ex
This also makes _console_buffer not overallocate where it would previously have space for these borders

Note: the change done for a bit of extra "speed" in the fairly hot (display) loop of __console_vipostcb made me notice this entire part is kind of flimsy when working with multiple PrintConsole instances at once, but that's a new thing so most probably not really used by anyone yet?
Useful if you want multiple fonts or wholly separate windows though, but maybe only in direct framebuffer mode then?

Copy link
Copy Markdown
Member

@DacoTaco DacoTaco left a comment

Choose a reason for hiding this comment

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

lgtm

@WinterMute
Copy link
Copy Markdown
Member

Note: the change done for a bit of extra "speed" in the fairly hot (display) loop of __console_vipostcb made me notice this entire part is kind of flimsy when working with multiple PrintConsole instances at once, but that's a new thing so most probably not really used by anyone yet? Useful if you want multiple fonts or wholly separate windows though, but maybe only in direct framebuffer mode then?

The intent of multiple PrintConsole instances is separate windows to the same underlying framebuffer. See for instance https://github.com/devkitPro/3ds-examples/blob/master/graphics/printing/multiple-windows-text/source/main.c . Each window could also have different fonts.

The intent of CON_InitEx is to overwrite part of the framebuffer for the current frame with a small console that could, for example, be used for debug output. I'm not sure if it makes sense for the PrintConsole API to be mixed with that but we can mull that over later.

Thanks for all your work on this @LiquidFenrir. These changes lgtm

@DacoTaco DacoTaco changed the title console: Further fixes for console writing out of bounds console: fix writing out of bounds and writing unused memory Sep 3, 2025
@DacoTaco DacoTaco merged commit 8c4a709 into devkitPro:master Sep 3, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants