Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
116 changes: 74 additions & 42 deletions src/backends/sdl2_renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@
}
}

void showDriverInUse(const Context &ctx) {
void showDriverInUse(Context &ctx) {
ctx.mLogger(LogSeverity::Message, "Driver in use:");
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_RendererInfo info;
SDL_GetRendererInfo(ctx.mSDLContext->mRenderer, &info);
SDL_GetRendererInfo(sdlCtx->mRenderer, &info);
printDriverInfo(info);
}

Expand Down Expand Up @@ -178,15 +179,17 @@
delete ctx.mDefaultFont;
ctx.mDefaultFont = nullptr;
}

IMG_Quit();
if (ctx.mSDLContext == nullptr) {
if (ctx.mBackendCtx == nullptr) {
SDL_Quit();
return ResultOk;
}

if (ctx.mSDLContext->mRenderer != nullptr) {
SDL_DestroyRenderer(ctx.mSDLContext->mRenderer);
ctx.mSDLContext->mRenderer = nullptr;
SDLContext *sdlCtx = getBackendContext(ctx);
if (sdlCtx->mRenderer != nullptr) {

Check warning on line 190 in src/backends/sdl2_renderer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use the init-statement to declare "sdlCtx" inside the if statement.

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgO&open=AZ0AzB5X0l_0ek4iPfgO&pullRequest=26
SDL_DestroyRenderer(sdlCtx->mRenderer);
sdlCtx->mRenderer = nullptr;
}
SDL_Quit();

Expand Down Expand Up @@ -220,15 +223,16 @@
return ErrorCode;
}

SDL_Texture *messageTexture = SDL_CreateTextureFromSurface(ctx.mSDLContext->mRenderer, surfaceMessage);
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_Texture *messageTexture = SDL_CreateTextureFromSurface(sdlCtx->mRenderer, surfaceMessage);
Comment on lines +226 to +227
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing null check on sdlCtx before dereferencing.

If getBackendContext(ctx) returns nullptr (e.g., backend not initialized), line 227 and 257 will dereference a null pointer.

🛡️ Proposed fix
     SDLContext *sdlCtx = getBackendContext(ctx);
+    if (sdlCtx == nullptr) {
+        return ErrorCode;
+    }
     SDL_Texture *messageTexture = SDL_CreateTextureFromSurface(sdlCtx->mRenderer, surfaceMessage);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 226 - 227,
getBackendContext(ctx) may return nullptr so add a null check for SDLContext*
sdlCtx before dereferencing; if sdlCtx is nullptr, handle the error (e.g.,
return an error code or clean up and exit the function) and avoid calling
SDL_CreateTextureFromSurface using sdlCtx->mRenderer (and similarly avoid other
uses of sdlCtx later). Specifically, guard the SDLContext* sdlCtx =
getBackendContext(ctx) result, log an appropriate error message, and
early-return to prevent dereferencing mRenderer in the
SDL_CreateTextureFromSurface call and any subsequent accesses.

if (messageTexture == nullptr) {
const std::string msg = "Cannot create texture: " + std::string(SDL_GetError()) + ".";
ctx.mLogger(LogSeverity::Error, msg.c_str());
return ErrorCode;
}

int32_t margin = ctx.mStyle.mMargin;
SDL_Rect Message_rect = {};
int32_t margin{ctx.mStyle.mMargin};
SDL_Rect Message_rect{};
switch (alignment) {
case Alignment::Left:
Message_rect.x = r.top.x + margin;
Expand All @@ -239,18 +243,18 @@
case Alignment::Center:
Message_rect.x = r.top.x + 2 * margin + surfaceMessage->clip_rect.w / 2;
Message_rect.y = r.top.y + margin;
Message_rect.w = font->mSize;
Message_rect.h = font->mSize;
Message_rect.w = font->mSize * static_cast<int>(strlen(string));
Message_rect.h = font->mSize + margin * 2;
break;
case Alignment::Right:
Message_rect.x = r.top.x + surfaceMessage->clip_rect.w - font->mSize * static_cast<int>(strlen(string));
Message_rect.y = r.top.y + margin;
Message_rect.w = font->mSize;
Message_rect.h = font->mSize;
Message_rect.w = font->mSize * static_cast<int>(strlen(string));
Message_rect.h = font->mSize + margin * 2;
break;
}

SDL_RenderCopy(ctx.mSDLContext->mRenderer, messageTexture, nullptr, &Message_rect);
SDL_RenderCopy(sdlCtx->mRenderer, messageTexture, nullptr, &Message_rect);
SDL_FreeSurface(surfaceMessage);
SDL_DestroyTexture(messageTexture);

Expand All @@ -259,15 +263,18 @@

ret_code Renderer::initScreen(Context &ctx, int32_t x, int32_t y, int32_t w, int32_t h) {
if (!ctx.mCreated) {
ctx.mLogger(LogSeverity::Error, "Not initialzed.");
ctx.mLogger(LogSeverity::Error, "Not initialized.");
return ErrorCode;
}
ctx.mSDLContext = SDLContext::create();
if (ctx.mSDLContext->mWindow != nullptr) {
SDLContext *sdlCtx = SDLContext::create();
if (sdlCtx->mWindow != nullptr) {
ctx.mLogger(LogSeverity::Error, "Already created.");

Check failure on line 271 in src/backends/sdl2_renderer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Potential leak of memory pointed to by 'sdlCtx'

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgS&open=AZ0AzB5X0l_0ek4iPfgS&pullRequest=26
return ErrorCode;
}

ctx.mBackendCtx = new BackendContext;

Check failure on line 275 in src/backends/sdl2_renderer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the use of "new" with an operation that automatically manages the memory.

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgP&open=AZ0AzB5X0l_0ek4iPfgP&pullRequest=26
ctx.mBackendCtx->mHandle = (void*) sdlCtx;
Comment on lines +275 to +276
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and get its size
wc -l src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 96


🏁 Script executed:

# Read the context around lines 275-330 to see all early returns
sed -n '270,330p' src/backends/sdl2_renderer.cpp | cat -n

Repository: kimkulling/tiny_ui

Length of output: 2490


🏁 Script executed:

# Look for the function signature and overall structure
grep -n "BackendContext\|SDLContext\|SDL2Renderer\|init\|Create" src/backends/sdl2_renderer.cpp | head -30

Repository: kimkulling/tiny_ui

Length of output: 1877


🏁 Script executed:

# Get the complete initScreen function starting from line 264
sed -n '264,327p' src/backends/sdl2_renderer.cpp | cat -n

Repository: kimkulling/tiny_ui

Length of output: 2591


🏁 Script executed:

# Search for SDLContext definition and destroy method
grep -n "struct SDLContext\|class SDLContext\|SDLContext::create\|SDLContext::destroy\|mBackendCtx" src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 312


🏁 Script executed:

# Find BackendContext definition
grep -rn "struct BackendContext\|class BackendContext" src/

Repository: kimkulling/tiny_ui

Length of output: 102


🏁 Script executed:

# Check if there's a destructor or cleanup helper
grep -n "~SDLContext\|~BackendContext\|destroy\|cleanup" src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 44


🏁 Script executed:

# Check BackendContext definition in tinyui.h
sed -n '384,400p' src/tinyui.h | cat -n

Repository: kimkulling/tiny_ui

Length of output: 1166


🏁 Script executed:

# Search for SDLContext definition
find src -type f \( -name "*.h" -o -name "*.hpp" -o -name "*.cpp" \) -exec grep -l "struct SDLContext\|class SDLContext" {} \;

Repository: kimkulling/tiny_ui

Length of output: 103


🏁 Script executed:

# Search for SDLContext::create and destroy implementations
grep -rn "SDLContext::create\|SDLContext::destroy\|~SDLContext" src/

Repository: kimkulling/tiny_ui

Length of output: 143


🏁 Script executed:

# Look for SDLContext definition
grep -B5 -A10 "struct SDLContext\|class SDLContext" src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 44


🏁 Script executed:

# Search for SDLContext in header files
cat src/backends/sdl2_renderer.h

Repository: kimkulling/tiny_ui

Length of output: 4264


🏁 Script executed:

# Search more broadly for SDLContext
rg "SDLContext" src/ -A 5 -B 5 | head -100

Repository: kimkulling/tiny_ui

Length of output: 5407


🏁 Script executed:

# Check the other initScreen overload (context version)
sed -n '328,365p' src/backends/sdl2_renderer.cpp | cat -n

Repository: kimkulling/tiny_ui

Length of output: 1306


🏁 Script executed:

# Check releaseScreen to see cleanup patterns
grep -A 20 "releaseScreen" src/backends/sdl2_renderer.cpp | head -40

Repository: kimkulling/tiny_ui

Length of output: 672


🏁 Script executed:

# Check if mOwner is set anywhere in the codebase
grep -n "mOwner" src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 93


🏁 Script executed:

# Verify mOwner usage in initScreen more carefully
sed -n '264,325p' src/backends/sdl2_renderer.cpp | grep -n "mOwner"

Repository: kimkulling/tiny_ui

Length of output: 44


🏁 Script executed:

# Check how destroy() is actually used
grep -n "\.destroy()" src/backends/sdl2_renderer.cpp

Repository: kimkulling/tiny_ui

Length of output: 44


Fix memory leaks on early returns in initScreen initialization.

After line 275-276 allocate BackendContext and SDLContext, multiple early returns (TTF_Init, loadDefaultFont, SDL_CreateWindow, driver query, SDL_CreateRenderer, surface acquisition) exit without cleanup, leaking both allocations. Additionally, line 273 has an early return that leaks sdlCtx before BackendContext is even allocated.

Implement a cleanup path that deletes BackendContext and calls sdlCtx->destroy() on all failure paths. Note: also set sdlCtx->mOwner = true on successful initialization so that destroy() properly cleans up SDL resources (window/renderer).

Lines affected by this issue
  • Line 273: Early return before BackendContext allocation
  • Lines 278, 283, 297, 303, 310, 322: Early returns after BackendContext allocation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 275 - 276, The initScreen path
leaks BackendContext and SDLContext on multiple early returns; ensure any early
return cleans up: when you create sdlCtx (SDLContext) but fail before allocating
ctx.mBackendCtx, call sdlCtx->destroy() and delete sdlCtx (or otherwise free it)
before returning; after allocating ctx.mBackendCtx and assigning mHandle, on
every subsequent failure (TTF_Init, loadDefaultFont, SDL_CreateWindow, driver
query/getDriver, SDL_CreateRenderer, surface acquisition) call
sdlCtx->destroy(), delete ctx.mBackendCtx (and set ctx.mBackendCtx = nullptr),
and return; on full successful init set sdlCtx->mOwner = true so
sdlCtx->destroy() will later clean up SDL window/renderer. Ensure all cleanup is
symmetric and idempotent (safe to call destroy/delete even if partially
initialized).


if (TTF_Init() == -1) {
ctx.mLogger(LogSeverity::Error, "TTF init failed.");
return ErrorCode;
Expand All @@ -283,8 +290,8 @@
title = "TinyUI Window";
}

ctx.mSDLContext->mWindow = SDL_CreateWindow(title, x, y, w, h, SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
if (ctx.mSDLContext->mWindow == nullptr) {
sdlCtx->mWindow = SDL_CreateWindow(title, x, y, w, h, SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
if (sdlCtx->mWindow == nullptr) {
const std::string msg = "Error while SDL_CreateWindow: " + std::string(SDL_GetError()) + ".";
ctx.mLogger(LogSeverity::Error, msg.c_str());
return ErrorCode;
Expand All @@ -296,8 +303,8 @@
return ErrorCode;
}

ctx.mSDLContext->mRenderer = SDL_CreateRenderer(ctx.mSDLContext->mWindow, driverIndex, SDL_RENDERER_ACCELERATED);
if (nullptr == ctx.mSDLContext->mRenderer) {
sdlCtx->mRenderer = SDL_CreateRenderer(sdlCtx->mWindow, driverIndex, SDL_RENDERER_ACCELERATED);
if (nullptr == sdlCtx->mRenderer) {
const std::string msg = "Error while SDL_CreateRenderer: " + std::string(SDL_GetError()) + ".";
ctx.mLogger(LogSeverity::Error, msg.c_str());
return ErrorCode;
Expand All @@ -309,8 +316,8 @@

showDriverInUse(ctx);

ctx.mSDLContext->mSurface = SDL_GetWindowSurface(ctx.mSDLContext->mWindow);
if (ctx.mSDLContext->mSurface == nullptr) {
sdlCtx->mSurface = SDL_GetWindowSurface(sdlCtx->mWindow);
if (sdlCtx->mSurface == nullptr) {
ctx.mLogger(LogSeverity::Error, "Surface pointer from window is nullptr.");
return ErrorCode;
}
Expand All @@ -320,7 +327,7 @@

ret_code Renderer::initScreen(Context &ctx, SDL_Window *window, SDL_Renderer *renderer) {
if (!ctx.mCreated) {
ctx.mLogger(LogSeverity::Error, "Not initialzed.");
ctx.mLogger(LogSeverity::Error, "TinyUI context is not initialzed.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo: "initialzed" should be "initialized".

The error message contains a spelling error.

✏️ Proposed fix
-        ctx.mLogger(LogSeverity::Error, "TinyUI context is not initialzed.");
+        ctx.mLogger(LogSeverity::Error, "TinyUI context is not initialized.");

Note: Line 361 has the same typo: "Not initialzed." should be "Not initialized."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ctx.mLogger(LogSeverity::Error, "TinyUI context is not initialzed.");
ctx.mLogger(LogSeverity::Error, "TinyUI context is not initialized.");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` at line 330, Fix the spelling mistake in the
TinyUI error messages by replacing "initialzed" with "initialized" in the logger
calls: update the string passed to ctx.mLogger(LogSeverity::Error, "...") at the
occurrence that reads "TinyUI context is not initialzed." and also the other
occurrence that reads "Not initialzed." so both messages read "initialized"
(search for ctx.mLogger and the exact strings to locate both spots).

return ErrorCode;
}

Expand All @@ -331,13 +338,19 @@

TTF_Init();

ctx.mSDLContext->mRenderer = renderer;
ctx.mSDLContext->mWindow = window;
SDLContext *sdlCtx = getBackendContext(ctx);
if (sdlCtx == nullptr) {
ctx.mLogger(LogSeverity::Error, "Invalid sdl context detected.");
return ErrorCode;
}

sdlCtx->mRenderer = renderer;
sdlCtx->mWindow = window;

showDriverInUse(ctx);

ctx.mSDLContext->mSurface = SDL_GetWindowSurface(ctx.mSDLContext->mWindow);
ctx.mSDLContext->mOwner = false;
sdlCtx->mSurface = SDL_GetWindowSurface(sdlCtx->mWindow);
sdlCtx->mOwner = false;
Comment thread
kimkulling marked this conversation as resolved.
ctx.mCreated = true;

return ResultOk;
Expand All @@ -349,8 +362,9 @@
return ErrorCode;
}

SDL_DestroyWindow(ctx.mSDLContext->mWindow);
ctx.mSDLContext->mWindow = nullptr;
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_DestroyWindow(sdlCtx->mWindow);
sdlCtx->mWindow = nullptr;

Comment on lines +365 to 368
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing null check on sdlCtx before dereferencing.

If getBackendContext(ctx) returns nullptr, line 366 will dereference a null pointer.

🛡️ Proposed fix
     SDLContext *sdlCtx = getBackendContext(ctx);
+    if (sdlCtx == nullptr) {
+        return ErrorCode;
+    }
     SDL_DestroyWindow(sdlCtx->mWindow);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_DestroyWindow(sdlCtx->mWindow);
sdlCtx->mWindow = nullptr;
SDLContext *sdlCtx = getBackendContext(ctx);
if (sdlCtx == nullptr) {
return ErrorCode;
}
SDL_DestroyWindow(sdlCtx->mWindow);
sdlCtx->mWindow = nullptr;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 365 - 368,
getBackendContext(ctx) may return nullptr, and the current code immediately
dereferences sdlCtx (SDLContext *sdlCtx = getBackendContext(ctx);
SDL_DestroyWindow(sdlCtx->mWindow);), which can crash; add a null check for
sdlCtx before accessing sdlCtx->mWindow and calling SDL_DestroyWindow (e.g., if
sdlCtx == nullptr then return or handle the error), and ensure you only set
sdlCtx->mWindow = nullptr after SDL_DestroyWindow is called on a valid sdlCtx.

return ResultOk;
}
Expand All @@ -362,19 +376,21 @@
}

const SDL_Color sdl_bg = getSDLColor(bg);
SDL_SetRenderDrawColor(ctx.mSDLContext->mRenderer, sdl_bg.r, sdl_bg.g, sdl_bg.b, sdl_bg.a);
SDL_RenderClear(ctx.mSDLContext->mRenderer);
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_SetRenderDrawColor(sdlCtx->mRenderer, sdl_bg.r, sdl_bg.g, sdl_bg.b, sdl_bg.a);
SDL_RenderClear(sdlCtx->mRenderer);

return ResultOk;
}

ret_code Renderer::drawRect(Context &ctx, int32_t x, int32_t y, int32_t w, int32_t h, bool filled, Color4 fg) {
SDL_Rect r = {x, y, w, h};
SDL_SetRenderDrawColor(ctx.mSDLContext->mRenderer, fg.r, fg.g, fg.b, fg.a);
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_SetRenderDrawColor(sdlCtx->mRenderer, fg.r, fg.g, fg.b, fg.a);
if (filled) {
SDL_RenderFillRect(ctx.mSDLContext->mRenderer, &r);
SDL_RenderFillRect(sdlCtx->mRenderer, &r);
} else {
SDL_RenderDrawRect(ctx.mSDLContext->mRenderer, &r);
SDL_RenderDrawRect(sdlCtx->mRenderer, &r);
}
Comment on lines 386 to 394
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing initialization and null checks.

Unlike other render functions, drawRect has no ctx.mCreated check and no null check on sdlCtx. This could crash if called before initialization or if backend setup failed.

🛡️ Proposed fix
 ret_code Renderer::drawRect(Context &ctx, int32_t x, int32_t y, int32_t w, int32_t h, bool filled, Color4 fg) {
+    if (!ctx.mCreated) {
+        return ErrorCode;
+    }
     SDL_Rect r = {x, y, w, h};
     SDLContext *sdlCtx = getBackendContext(ctx);
+    if (sdlCtx == nullptr) {
+        return ErrorCode;
+    }
     SDL_SetRenderDrawColor(sdlCtx->mRenderer, fg.r, fg.g, fg.b, fg.a);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 386 - 394, drawRect lacks the
usual initialization guards: add a check at the top of Renderer::drawRect to
verify ctx.mCreated and return the same error ret_code used by other render
functions when not created, then call getBackendContext(ctx) and check the
returned SDLContext* (sdlCtx) for null and return the same backend-failure
ret_code used elsewhere if null; only proceed to set color and call
SDL_RenderFillRect/SDL_RenderDrawRect after those checks so drawRect mirrors the
safety of the other renderer methods.


return ResultOk;
Expand All @@ -386,28 +402,31 @@
}

SDL_Rect imageRect = {x, y, w, h};
SDL_Texture *tex = SDL_CreateTextureFromSurface(ctx.mSDLContext->mRenderer, image->mSurfaceImpl->mSurface);
SDL_RenderCopy(ctx.mSDLContext->mRenderer, tex, nullptr, &imageRect);
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_Texture *tex = SDL_CreateTextureFromSurface(sdlCtx->mRenderer, image->mSurfaceImpl->mSurface);
SDL_RenderCopy(sdlCtx->mRenderer, tex, nullptr, &imageRect);
SDL_DestroyTexture(tex);

return ResultOk;
}

ret_code Renderer::closeScreen(Context &ctx) {
if (ctx.mSDLContext->mWindow == nullptr) {
SDLContext *sdlCtx = getBackendContext(ctx);
if (sdlCtx->mWindow == nullptr) {
return ErrorCode;
}

TTF_Quit();

SDL_DestroyWindow(ctx.mSDLContext->mWindow);
ctx.mSDLContext->mWindow = nullptr;
SDL_DestroyWindow(sdlCtx->mWindow);
sdlCtx->mWindow = nullptr;

return ResultOk;
}
Comment on lines 413 to 425
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Resource leak: closeScreen does not clean up SDLContext or BackendContext.

The function destroys mWindow but does not:

  1. Destroy the SDL renderer (mRenderer)
  2. Free the SDLContext heap object
  3. Free the BackendContext wrapper
  4. Set ctx.mBackendCtx to nullptr

As noted in the context snippets, SDLContext::destroy() exists but is never called.

🛡️ Proposed fix: Complete cleanup
 ret_code Renderer::closeScreen(Context &ctx) {
     SDLContext *sdlCtx = getBackendContext(ctx);
+    if (sdlCtx == nullptr) {
+        return ErrorCode;
+    }
     if (sdlCtx->mWindow == nullptr) {
         return ErrorCode;
     }
 
     TTF_Quit();
 
-    SDL_DestroyWindow(sdlCtx->mWindow);
-    sdlCtx->mWindow = nullptr;
+    sdlCtx->mOwner = true;  // Take ownership for cleanup
+    sdlCtx->destroy();      // Destroys renderer, window, and frees SDLContext
+    
+    delete ctx.mBackendCtx;
+    ctx.mBackendCtx = nullptr;
 
     return ResultOk;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 407 - 419, closeScreen currently
only destroys mWindow and quits TTF but leaks the SDL renderer and heap
contexts; update Renderer::closeScreen to call SDLContext::destroy() (which
should handle destroying mRenderer and any SDL subresources), then free/delete
the SDLContext instance and the BackendContext wrapper retrieved via
getBackendContext(ctx), and finally set ctx.mBackendCtx to nullptr; ensure you
still call TTF_Quit() and set sdlCtx->mWindow = nullptr (or rely on
SDLContext::destroy() to clear members) but do not return before cleaning up the
BackendContext object.


ret_code Renderer::endRender(Context &ctx) {
SDL_RenderPresent(ctx.mSDLContext->mRenderer);
SDLContext *sdlCtx = getBackendContext(ctx);
SDL_RenderPresent(sdlCtx->mRenderer);

return ResultOk;
}
Expand All @@ -416,7 +435,8 @@
if (texture == nullptr) {
return ErrorCode;
}
*texture = SDL_CreateTexture(ctx.mSDLContext->mRenderer, SDL_PIXELFORMAT_RGBA8888, SDL_TEXTUREACCESS_TARGET, w, h);
SDLContext *sdlCtx = getBackendContext(ctx);
*texture = SDL_CreateTexture(sdlCtx->mRenderer, SDL_PIXELFORMAT_RGBA8888, SDL_TEXTUREACCESS_TARGET, w, h);

return ResultOk;
}
Expand Down Expand Up @@ -499,4 +519,16 @@
}
}

ret_code Renderer::getSurfaceInfo(Context &ctx, int32_t &w, int32_t &h) {
SDLContext *sdlCtx = (SDLContext *) ctx.mBackendCtx;

Check warning on line 523 in src/backends/sdl2_renderer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the redundant type with "auto".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgQ&open=AZ0AzB5X0l_0ek4iPfgQ&pullRequest=26

Check warning on line 523 in src/backends/sdl2_renderer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Make the type of this variable a pointer-to-const. The current type of "sdlCtx" is "struct tinyui::SDLContext *".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgR&open=AZ0AzB5X0l_0ek4iPfgR&pullRequest=26
if (sdlCtx->mSurface == nullptr) {
return ErrorCode;
}

w = sdlCtx->mSurface->w;
h = sdlCtx->mSurface->h;

return ResultOk;
}
Comment on lines +522 to +532
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical bug: Incorrect cast in getSurfaceInfo — casts mBackendCtx instead of mBackendCtx->mHandle.

Line 517 casts ctx.mBackendCtx directly to SDLContext*, but mBackendCtx is a BackendContext*, not the SDL handle. The correct cast should be on mBackendCtx->mHandle, as done in getBackendContext().

This will cause undefined behavior when accessing sdlCtx->mSurface.

🐛 Proposed fix
 ret_code Renderer::getSurfaceInfo(Context &ctx, int32_t &w, int32_t &h) {
-    SDLContext *sdlCtx = (SDLContext *) ctx.mBackendCtx;
+    if (ctx.mBackendCtx == nullptr) {
+        return ErrorCode;
+    }
+    const auto *sdlCtx = static_cast<const SDLContext*>(ctx.mBackendCtx->mHandle);
     if (sdlCtx->mSurface == nullptr) {
         return ErrorCode;
     }
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis

[warning] 517-517: Replace the redundant type with "auto".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgQ&open=AZ0AzB5X0l_0ek4iPfgQ&pullRequest=26


[warning] 517-517: Make the type of this variable a pointer-to-const. The current type of "sdlCtx" is "struct tinyui::SDLContext *".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB5X0l_0ek4iPfgR&open=AZ0AzB5X0l_0ek4iPfgR&pullRequest=26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.cpp` around lines 516 - 526, In getSurfaceInfo,
you are incorrectly casting ctx.mBackendCtx to SDLContext*; instead retrieve the
BackendContext first and cast its mHandle to SDLContext* (same pattern as
getBackendContext()), check that the BackendContext and its mHandle/mSurface are
non-null before accessing mSurface, and return ErrorCode on nulls; update
references to use the SDLContext* derived from ctx.mBackendCtx->mHandle.


} // namespace tinyui
5 changes: 5 additions & 0 deletions src/backends/sdl2_renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@
}
};

inline SDLContext *getBackendContext(Context &ctx) {
SDLContext *sdlCtx = (SDLContext*) ctx.mBackendCtx->mHandle;

Check warning on line 104 in src/backends/sdl2_renderer.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the redundant type with "auto".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB7h0l_0ek4iPfgT&open=AZ0AzB7h0l_0ek4iPfgT&pullRequest=26
return sdlCtx;
}
Comment on lines +103 to +106
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

getBackendContext() lacks null-safety and uses C-style cast.

This helper dereferences ctx.mBackendCtx->mHandle without checking if either pointer is null. As shown in the context snippets, initScreen can fail after allocating mBackendCtx but before fully initializing it, leaving mHandle valid but pointing to an incompletely initialized SDLContext.

Additionally, prefer static_cast over C-style casts per C++ best practices.

🛡️ Proposed fix with null checks and proper cast
 inline SDLContext *getBackendContext(Context &ctx) {
-    SDLContext *sdlCtx = (SDLContext*) ctx.mBackendCtx->mHandle;
-    return sdlCtx;
+    if (ctx.mBackendCtx == nullptr) {
+        return nullptr;
+    }
+    return static_cast<SDLContext*>(ctx.mBackendCtx->mHandle);
 }

Note: All callers of getBackendContext() should then check for a null return value before dereferencing.

🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis

[warning] 104-104: Replace the redundant type with "auto".

See more on https://sonarcloud.io/project/issues?id=kimkulling_tiny_ui&issues=AZ0AzB7h0l_0ek4iPfgT&open=AZ0AzB7h0l_0ek4iPfgT&pullRequest=26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backends/sdl2_renderer.h` around lines 103 - 106, getBackendContext
currently uses a C-style cast and dereferences ctx.mBackendCtx and its mHandle
without null checks; change it to verify ctx.mBackendCtx and
ctx.mBackendCtx->mHandle are non-null and return nullptr if either is null, and
replace the C-style cast with a static_cast<SDLContext*> when converting mHandle
to SDLContext*; update callers (e.g., places that assume getBackendContext()
always returns valid) to handle a nullptr return before dereferencing.


/// @brief The renderer implementation using the SDL2 library.
struct Renderer {
Expand All @@ -118,6 +122,7 @@
static bool update(Context &ctx);
static SurfaceImpl *createSurfaceImpl(unsigned char *data, int w, int h, int bytesPerPixel, int pitch);
static void releaseSurfaceImpl(SurfaceImpl *surfaceImpl);
static ret_code getSurfaceInfo(Context &ctx, int32_t &w, int32_t &h);
};

} // namespace tinyui
34 changes: 12 additions & 22 deletions src/tinyui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ SOFTWARE.
#include "backends/sdl2_iodevice.h"

#include <iostream>
#include <cstring>

namespace tinyui {

Expand All @@ -37,7 +36,7 @@ static Style DefaultStyle {
Color4{ 20, 20, 20, 0 },
Color4{ 100, 100, 100, 0 },
Color4{ 200, 200, 200, 0 },
8,
2,
{ "Arial.ttf", 12, nullptr }
};

Expand Down Expand Up @@ -98,7 +97,7 @@ Context &TinyUi::getContext() {
}

ret_code TinyUi::initScreen(int32_t x, int32_t y, int32_t w, int32_t h) {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
if (Renderer::initRenderer(ctx) == ErrorCode) {
printf("Error: Cannot init renderer\n");
return ErrorCode;
Expand All @@ -114,14 +113,7 @@ ret_code TinyUi::getSurfaceInfo(int32_t &w, int32_t &h) {
return ErrorCode;
}

if (ctx.mSDLContext->mSurface == nullptr) {
return ErrorCode;
}

w = ctx.mSDLContext->mSurface->w;
h = ctx.mSDLContext->mSurface->h;

return ResultOk;
return Renderer::getSurfaceInfo(ctx, w, h);
}

ret_code TinyUi::getSurfaceCenter(int32_t &x, int32_t &y) {
Expand All @@ -138,7 +130,7 @@ ret_code TinyUi::getSurfaceCenter(int32_t &x, int32_t &y) {
}

bool TinyUi::run() {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
if (!ctx.mUpdateCallbackList.empty()) {
for (auto it = ctx.mUpdateCallbackList.begin(); it != ctx.mUpdateCallbackList.end(); ++it) {
(*it)->mfuncCallback[Events::UpdateEvent](1, (*it)->mInstance);
Expand All @@ -148,31 +140,29 @@ bool TinyUi::run() {
}

ret_code TinyUi::beginRender(Color4 bg) {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
return Renderer::beginRender(ctx, bg);
}

ret_code TinyUi::endRender() {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
return Renderer::endRender(ctx);
}

void TinyUi::render() {
auto &ctx = TinyUi::getContext();
TinyUi::beginRender(ctx.mStyle.mClearColor);
auto &ctx = getContext();
beginRender(ctx.mStyle.mClearColor);
Widgets::renderWidgets();
TinyUi::endRender();
endRender();
}

ret_code TinyUi::release() {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
if (!ctx.mCreated) {
return ErrorCode;
}
Renderer::releaseRenderer(ctx);
Renderer::releaseScreen(ctx);
ctx.mSDLContext->mRenderer = nullptr;
ctx.mSDLContext->mWindow = nullptr;
ctx.mRoot = nullptr;

ctx.mCreated = false;
Expand All @@ -193,12 +183,12 @@ void TinyUi::setDefaultStyle(const Style &style) {
}

void TinyUi::setDefaultFont(const char *defaultFont) {
auto &ctx = TinyUi::getContext();
auto &ctx = getContext();
if (defaultFont == nullptr) {
return;
}

if (strncmp( ctx.mStyle.mFont.mName, defaultFont, std::strlen(defaultFont)) == 0) {
if (strncmp( ctx.mStyle.mFont.mName, defaultFont, strlen(defaultFont)) == 0) {
return;
}

Expand Down
Loading
Loading