feat: add SDL support#227
Conversation
|
facing this issue as well: lvgl/lvgl#9237 |
|
does this enable iterating on the UI without flashing a device ? |
yes, but that can be done with native/X11, too |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLVGL is upgraded to 9.5 with expanded configuration, SDL display support is added, platform-specific timing is revised, and TFT UI styling is updated across maps, nodes, messages, connectivity indicators, and chat controls. ChangesLVGL display updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DisplayDriverFactory
participant SDLDriver
participant DisplayDriver
participant LVGL
DisplayDriverFactory->>SDLDriver: create(width, height)
SDLDriver->>DisplayDriver: initialize base driver
SDLDriver->>LVGL: create SDL display and input devices
SDLDriver->>DisplayDriver: run display task handler
SDLDriver->>LVGL: advance LVGL ticks
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
library.json (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the LVGL version banner in
include/lv_conf.h
library.jsonpins LVGLv9.5.0, but the header still saysv9.2.2. Keep the comment aligned with the dependency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@library.json` at line 24, Update the LVGL version banner comment in lv_conf.h to state v9.5.0, matching the version pinned by the lvgl/lvgl dependency in library.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@include/graphics/driver/SDLDriver.h`:
- Around line 4-9: Update the documentation comment above the SDL driver class
to reference SDL2 linking, replacing the misleading -lSDL flag with -lSDL2 or
the project’s established SDL2 build configuration. Keep the existing usage and
platform guidance unchanged.
In `@include/lv_conf.h`:
- Around line 288-292: Update the SDL and X11 driver preprocessor guards to use
value-based checks, replacing `#ifdef/`#if defined(...) checks with `#if` USE_SDL or
`#if` USE_X11 as appropriate. Ensure USE_SDL=0 and USE_X11=0 disable their
respective paths while nonzero values enable them, including the related guards
in include/lv_conf.h.
In `@source/graphics/driver/X11Driver.cpp`:
- Around line 32-49: Update X11Driver::task_handler to remove both lv_tick_inc()
calls while preserving the existing duration-based sleep behavior in both
branches.
In `@source/graphics/TFT/TFTView_320x240.cpp`:
- Around line 7144-7146: Replace the unsafe strcpy call in the no-new-messages
branch with a bounded copy using snprintf or strlcpy, ensuring the translated
string is truncated safely and buf remains null-terminated within its 64-byte
capacity.
- Around line 5185-5187: Replace the sprintf call formatting the MAC address in
the Bluetooth label update with snprintf, passing the size of buf so writes are
bounded while preserving the existing output format and lv_label_set_text call.
- Line 244: Rename the LVGL style setter calls throughout the TFT view
implementation from the deprecated `bg_img` spelling to the LVGL 9 `bg_image`
spelling, including `lv_obj_set_style_bg_img_recolor` and all other `_img_`
style setters in this file. Preserve each call’s existing arguments and
behavior.
---
Nitpick comments:
In `@library.json`:
- Line 24: Update the LVGL version banner comment in lv_conf.h to state v9.5.0,
matching the version pinned by the lvgl/lvgl dependency in library.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c1b4c862-46f2-4655-a716-8bdfe1bc0464
📒 Files selected for processing (10)
include/graphics/driver/SDLDriver.hinclude/graphics/driver/TFTDriver.hinclude/graphics/driver/X11Driver.hinclude/lv_conf.hlibrary.jsonsource/graphics/DeviceGUI.cppsource/graphics/TFT/TFTView_320x240.cppsource/graphics/driver/DisplayDriverFactory.cppsource/graphics/driver/SDLDriver.cppsource/graphics/driver/X11Driver.cpp
💤 Files with no reviewable changes (1)
- source/graphics/DeviceGUI.cpp
| /** | ||
| * @brief For simulation on pc/raspberry | ||
| * This class provides an SDL GUI on the local desktop; dimensions are defined | ||
| * in lv_drv_conf.h Usage: define USE_SDL=1 for the rasbian/portduino target and | ||
| * link with -lSDL | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -- '-lSDL(2)?\b|sdl2-config|pkg-config.*sdl2|SDL2::SDL2' .Repository: meshtastic/device-ui
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SDLDriver.h =="
cat -n include/graphics/driver/SDLDriver.h
echo
echo "== SDL-related files =="
git ls-files | rg '(^|/)(CMakeLists\.txt|.*\.cmake|.*\.(h|hpp|c|cc|cpp))$' | rg 'SDL|sdl|graphics/driver'
echo
echo "== SDL includes/usages =="
rg -n --glob '!**/build/**' --glob '!**/dist/**' --glob '!**/node_modules/**' 'SDL2?|sdl2-config|pkg-config.*sdl2|SDL_' include src .github CMakeLists.txt *.cmake
echo
echo "== Build/link config mentions =="
rg -n --glob '!**/build/**' --glob '!**/dist/**' --glob '!**/node_modules/**' '-lSDL(2)?\b|SDL2::SDL2|PkgConfig::.*SDL2|find_package\(SDL2|find_package\(SDL' .Repository: meshtastic/device-ui
Length of output: 3592
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SDL driver implementation =="
cat -n source/graphics/driver/SDLDriver.cpp
echo
echo "== SDL2 and linker config mentions =="
rg -n --glob '!**/build/**' --glob '!**/dist/**' --glob '!**/node_modules/**' \
'SDL2|sdl2-config|pkg-config.*sdl2|find_package\(SDL|find_package\(SDL2|SDL2::SDL2|PkgConfig::.*SDL2|SDL_INCLUDE|SDL_LIB|USE_SDL|LV_USE_SDL' \
include source .
echo
echo "== Build files =="
git ls-files | rg '(^|/)(CMakeLists\.txt|.*\.cmake|Makefile|GNUmakefile|Kconfig|platformio\.ini)$'Repository: meshtastic/device-ui
Length of output: 4303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CMakeLists.txt =="
cat -n CMakeLists.txt
echo
echo "== cmake/Portduino.cmake =="
cat -n cmake/Portduino.cmake
echo
echo "== cmake/LVGL.cmake =="
cat -n cmake/LVGL.cmake
echo
echo "== SDL build references =="
rg -n --glob '!**/build/**' --glob '!**/dist/**' --glob '!**/node_modules/**' \
'SDL2|SDL|sdl2-config|pkg-config|target_link_libraries|find_package' CMakeLists.txt cmake include sourceRepository: meshtastic/device-ui
Length of output: 10711
Document the SDL2 linker flag
This code uses SDL2 (<SDL2/SDL.h> and LV_SDL_INCLUDE_PATH), so -lSDL is misleading. Update the note to point to SDL2 linking (-lSDL2 or the project’s SDL2 build config).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/graphics/driver/SDLDriver.h` around lines 4 - 9, Update the
documentation comment above the SDL driver class to reference SDL2 linking,
replacing the misleading -lSDL flag with -lSDL2 or the project’s established
SDL2 build configuration. Keep the existing usage and platform guidance
unchanged.
| #ifndef USE_SDL | ||
| #define LV_USE_SDL 0 | ||
| #else | ||
| #define LV_USE_SDL USE_SDL | ||
| #endif |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 '`#if`(n?def| defined).*USE_(SDL|X11)|`#if` +USE_(SDL|X11)' include sourceRepository: meshtastic/device-ui
Length of output: 3241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== lv_conf.h excerpt =='
sed -n '280,300p' include/lv_conf.h
echo
sed -n '1214,1230p' include/lv_conf.h
echo
echo '== macro usage =='
rg -n -C1 '\bUSE_(SDL|X11)\b|\bLV_USE_(SDL|X11)\b' include sourceRepository: meshtastic/device-ui
Length of output: 4779
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== DisplayDriverFactory.cpp relevant sections =='
sed -n '1,40p' source/graphics/driver/DisplayDriverFactory.cpp
echo
sed -n '88,140p' source/graphics/driver/DisplayDriverFactory.cpp
echo
sed -n '210,235p' source/graphics/driver/DisplayDriverFactory.cpp
echo
echo '== driver source guards =='
sed -n '1,20p' source/graphics/driver/SDLDriver.cpp
echo
sed -n '1,20p' source/graphics/driver/X11Driver.cppRepository: meshtastic/device-ui
Length of output: 4601
Use value-based SDL/X11 guards
USE_SDL and USE_X11 are treated as numeric feature flags in include/lv_conf.h, but the SDL/X11 driver code still uses #ifdef/#if defined(...). That makes USE_SDL=0 or USE_X11=0 still compile/select those paths. Switch those checks to #if USE_SDL / #if USE_X11 for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/lv_conf.h` around lines 288 - 292, Update the SDL and X11 driver
preprocessor guards to use value-based checks, replacing `#ifdef/`#if defined(...)
checks with `#if` USE_SDL or `#if` USE_X11 as appropriate. Ensure USE_SDL=0 and
USE_X11=0 disable their respective paths while nonzero values enable them,
including the related guards in include/lv_conf.h.
| /** | ||
| * Measure how long it takes to call DisplayDriver::task_handler(). | ||
| * Then tell the lvgl library how long it took via lv_tick_inc(). | ||
| */ | ||
| void X11Driver::task_handler(void) | ||
| { | ||
| const int ms = 10; | ||
| auto start = std::chrono::high_resolution_clock::now(); | ||
| DisplayDriver::task_handler(); | ||
| auto stop = std::chrono::high_resolution_clock::now(); | ||
| auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(stop - start); | ||
| if (duration.count() < ms) { | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(ms - duration.count())); | ||
| lv_tick_inc(ms); | ||
| } else { | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(1)); | ||
| lv_tick_inc(duration.count() + 1); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -fsSL \
https://raw.githubusercontent.com/lvgl/lvgl/v9.5.0/src/drivers/x11/lv_x11_display.c |
rg -n -C3 'x11_tick_thread|lv_tick_inc'Repository: meshtastic/device-ui
Length of output: 605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the driver and its LVGL/X11 integration points
git ls-files 'source/graphics/driver/*' 'source/graphics/*' 'src/*' 'include/*' | sed -n '1,200p'
echo '--- X11Driver.cpp ---'
cat -n source/graphics/driver/X11Driver.cpp | sed -n '1,220p'
echo '--- search for X11 backend / lv_tick_inc / lv_timer_handler usage ---'
rg -n 'lv_tick_inc|lv_timer_handler|x11|LV_USE_X11|lv_x11' source include srcRepository: meshtastic/device-ui
Length of output: 9173
Remove the extra X11 tick updates
LVGL’s X11 backend already advances time on its own 5 ms thread, so these lv_tick_inc() calls double-count time and make timers/animations run too fast. Keep the sleep, but remove both increments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/graphics/driver/X11Driver.cpp` around lines 32 - 49, Update
X11Driver::task_handler to remove both lv_tick_inc() calls while preserving the
existing duration-based sleep behavior in both branches.
| Themes::recolorText(objects.home_bell_label, false); | ||
|
|
||
| lv_obj_set_style_bg_img_recolor(objects.home_button, colorMesh, LV_PART_MAIN | LV_STATE_DEFAULT); | ||
| lv_obj_set_style_bg_img_recolor(objects.home_button, colorMesh, LV_PART_MAIN); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if LVGL provides definitions for both `img` and `image` variants of style properties.
echo "Checking for bg_img_recolor:"
rg "lv_obj_set_style_bg_img_recolor" -A 1 .pio/libdeps/*/lvgl/src/ || echo "Not found."
echo "Checking for bg_image_recolor:"
rg "lv_obj_set_style_bg_image_recolor" -A 1 .pio/libdeps/*/lvgl/src/ || echo "Not found."
echo "Checking for img_recolor vs image_recolor:"
rg "lv_obj_set_style_img_recolor\b" .pio/libdeps/*/lvgl/src/ || echo "Not found."Repository: meshtastic/device-ui
Length of output: 489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Top-level files:"
git ls-files | sed -n '1,120p'
echo
echo "Possible LVGL files:"
git ls-files | rg '(^|/)lvgl/|lvgl\.h$|lvgl' || true
echo
echo "Target file context:"
sed -n '230,255p' source/graphics/TFT/TFTView_320x240.cpp
echo
echo "Search for img/image style APIs in the repo:"
rg -n "lv_obj_set_style_(bg_)?(img|image)_(recolor|recolor_opa|src)\b|lv_obj_set_style_(img|image)_(recolor|recolor_opa)\b" .Repository: meshtastic/device-ui
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "LVGL version references:"
rg -n "LVGL|LV_VERSION|LVGL_VERSION|9\.[0-9]" CMakeLists.txt cmake extra_script.py README.md docs source generated || true
echo
echo "LVGL include / package wiring:"
sed -n '1,220p' cmake/LVGL.cmake
echo
echo "Other references to the same API family in source tree:"
rg -n "lv_obj_set_style_(bg_)?(img|image)_(recolor|recolor_opa|src)\b|lv_obj_set_style_(img|image)_(recolor|recolor_opa)\b" source generated | sed -n '1,120p'Repository: meshtastic/device-ui
Length of output: 50376
🌐 Web query:
LVGL 9 lv_obj_set_style_bg_img_recolor lv_obj_set_style_bg_image_recolor API rename image img
💡 Result:
In LVGL 9, the image recoloring API has been updated to use the naming convention image_... instead of img_... [1][2]. Specifically, the function previously known as lv_obj_set_style_bg_img_recolor has been renamed to lv_obj_set_style_bg_image_recolor [3][4]. Similarly, functions and properties related to general image recoloring have been updated to use the image prefix (e.g., lv_obj_set_style_image_recolor) [3][4][5]. This change is part of a broader refactoring in LVGL 9 where img_ prefixes were updated to image_ to maintain consistent naming [1][2]. When migrating or writing new code for LVGL 9, you should use the image_ prefix for all style-related recoloring functions [3][4].
Citations:
- 1: https://lvgl.io/docs/open/9.0/CHANGELOG
- 2: https://lvgl.github.io/open-docs/HTML/9.0/CHANGELOG.html
- 3: https://lvgl.io/docs/open/api/public/core/lv_obj_style_gen_h
- 4: https://lvgl.io/docs/open/api/public/core/lv_obj_style_h
- 5: https://lvgl.io/docs/open/9.5/widgets/image.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Exact _img_ style API uses:"
rg -n "\b(lv_obj_set_style_(bg_)?img_(recolor|recolor_opa|src)|lv_obj_set_style_img_(recolor|recolor_opa))\b" source generated || true
echo
echo "Exact _image_ style API uses:"
rg -n "\b(lv_obj_set_style_(bg_)?image_(recolor|recolor_opa|src)|lv_obj_set_style_image_(recolor|recolor_opa))\b" source generated | sed -n '1,80p'Repository: meshtastic/device-ui
Length of output: 16384
🌐 Web query:
site:lvgl.io "lv_obj_set_style_bg_img_recolor" OR "lv_obj_set_style_bg_image_recolor" LVGL 9
💡 Result:
In LVGL 9, style properties and their corresponding setter functions have been renamed to be more concise, generally omitting redundant prefixes like "bg_" or "img_" where the context of the property type is otherwise clear [1][2]. For recoloring, the relevant properties and functions are: 1. Image Widget Recoloring: The property is image_recolor and image_recolor_opa [2]. To set these on an object, use: - lv_obj_set_style_image_recolor(obj, color, selector) [3][4] - lv_obj_set_style_image_recolor_opa(obj, opa, selector) [3][4] 2. Background Image Recoloring: The property is bg_image_recolor and bg_image_recolor_opa [2]. To set these on an object, use: - lv_obj_set_style_bg_image_recolor(obj, color, selector) - lv_obj_set_style_bg_image_recolor_opa(obj, opa, selector) Note that in earlier versions of LVGL (such as 8.x), these properties were often named bg_img_recolor and img_recolor. In LVGL 9, they have been updated to bg_image_recolor and image_recolor [5][2][6]. Therefore, the functions lv_obj_set_style_bg_img_recolor and lv_obj_set_style_bg_image_recolor (and their variants) refer to the same logical operation, but you should use the updated naming convention (bg_image_recolor / image_recolor) consistent with the LVGL 9 API [2][7].
Citations:
- 1: https://lvgl.io/docs/open/9.2/overview/style
- 2: https://lvgl.io/docs/open/9.2/overview/style-props.html
- 3: https://lvgl.io/docs/open/9.2/widgets/image
- 4: https://lvgl.io/docs/open/9.5/widgets/image.html
- 5: https://lvgl.io/docs/open/9.0/overview/style-props
- 6: https://lvgl.io/docs/open/9.0/overview/style-props.html
- 7: https://lvgl.io/docs/open/9.1/overview/style-props
🌐 Web query:
site:docs.lvgl.io "lv_obj_set_style_bg_img_recolor" "lv_obj_set_style_bg_image_recolor"
💡 Result:
In LVGL (Light and Versatile Graphics Library), the two function names refer to the same functionality for styling images, but they belong to different versions of the library, reflecting a naming convention update. The correct function name is lv_obj_set_style_image_recolor [1]. Here is the distinction: 1. Evolution of Naming: In recent versions of LVGL (such as version 9.x), the library has standardized its naming conventions. The function lv_obj_set_style_image_recolor is the current, official API [1]. 2. Deprecated usage: Older naming variations, such as lv_obj_set_style_bg_img_recolor, are generally remnants of earlier versions or non-standard abbreviations that have been updated to be more explicit and consistent with the current API structure. 3. Purpose: Both names, regardless of the version, are used to set the recolor property for an image [1]. This allows you to mix a color with the pixels of an image, which is useful for indicating states (e.g., pressed, inactive, or checked) without needing to store multiple versions of the same image file [1]. If you are working with a modern version of LVGL (v8 or v9+), you should use lv_obj_set_style_image_recolor [1]. If you encounter lv_obj_set_style_bg_img_recolor in existing code, it is advisable to update it to the modern naming convention to ensure compatibility and consistency with the current documentation [1].
Citations:
Rename this to the LVGL 9 bg_image setter
lv_obj_set_style_bg_img_recolor(...) is the old spelling; LVGL 9 uses lv_obj_set_style_bg_image_recolor(...). The same rename applies to the other _img_ style setters in this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/graphics/TFT/TFTView_320x240.cpp` at line 244, Rename the LVGL style
setter calls throughout the TFT view implementation from the deprecated `bg_img`
spelling to the LVGL 9 `bg_image` spelling, including
`lv_obj_set_style_bg_img_recolor` and all other `_img_` style setters in this
file. Preserve each call’s existing arguments and behavior.
| sprintf(buf, "??:??:%02x:%02x:%02x:%02x", mac & 0xff, (mac & 0xff00) >> 8, (mac & 0xff0000) >> 16, | ||
| (mac & 0xff000000) >> 24); | ||
| lv_label_set_text(objects.home_bluetooth_label, buf); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use snprintf to prevent potential buffer overflows.
As highlighted by static analysis, using sprintf into a fixed-size buffer can overflow if the formatting assumptions are violated. Use snprintf to ensure safety.
🛡️ Proposed fix
- sprintf(buf, "??:??:%02x:%02x:%02x:%02x", mac & 0xff, (mac & 0xff00) >> 8, (mac & 0xff0000) >> 16,
- (mac & 0xff000000) >> 24);
+ snprintf(buf, sizeof(buf), "??:??:%02x:%02x:%02x:%02x", mac & 0xff, (mac & 0xff00) >> 8, (mac & 0xff0000) >> 16,
+ (mac & 0xff000000) >> 24);📝 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.
| sprintf(buf, "??:??:%02x:%02x:%02x:%02x", mac & 0xff, (mac & 0xff00) >> 8, (mac & 0xff0000) >> 16, | |
| (mac & 0xff000000) >> 24); | |
| lv_label_set_text(objects.home_bluetooth_label, buf); | |
| snprintf(buf, sizeof(buf), "??:??:%02x:%02x:%02x:%02x", mac & 0xff, (mac & 0xff00) >> 8, (mac & 0xff0000) >> 16, | |
| (mac & 0xff000000) >> 24); | |
| lv_label_set_text(objects.home_bluetooth_label, buf); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/graphics/TFT/TFTView_320x240.cpp` around lines 5185 - 5187, Replace
the sprintf call formatting the MAC address in the Bluetooth label update with
snprintf, passing the size of buf so writes are bounded while preserving the
existing output format and lv_label_set_text call.
Source: Linters/SAST tools
| } else { | ||
| strcpy(buf, _("no new messages")); | ||
| lv_obj_set_style_bg_img_src(objects.home_mail_button, &img_home_mail_button_image, LV_PART_MAIN | LV_STATE_DEFAULT); | ||
| lv_obj_set_style_bg_img_src(objects.home_mail_button, &img_home_mail_button_image, LV_PART_MAIN); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use snprintf or strlcpy instead of strcpy.
If the translated string returned by _("no new messages") exceeds the 64-byte size of buf, it will cause a buffer overflow.
🛡️ Proposed fix
- strcpy(buf, _("no new messages"));
+ snprintf(buf, sizeof(buf), "%s", _("no new messages"));📝 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.
| } else { | |
| strcpy(buf, _("no new messages")); | |
| lv_obj_set_style_bg_img_src(objects.home_mail_button, &img_home_mail_button_image, LV_PART_MAIN | LV_STATE_DEFAULT); | |
| lv_obj_set_style_bg_img_src(objects.home_mail_button, &img_home_mail_button_image, LV_PART_MAIN); | |
| } else { | |
| snprintf(buf, sizeof(buf), "%s", _("no new messages")); | |
| lv_obj_set_style_bg_img_src(objects.home_mail_button, &img_home_mail_button_image, LV_PART_MAIN); |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 7144-7144: Use of an unbounded buffer function that can overflow the destination; use a size-bounded equivalent (fgets, strncpy/strlcpy, strncat/strlcat, snprintf).
Context: strcpy(buf, _("no new messages"))
Note: [CWE-120] Buffer Copy without Checking Size of Input ('Classic Buffer Overflow').
(dangerous-buffer-functions-cpp)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/graphics/TFT/TFTView_320x240.cpp` around lines 7144 - 7146, Replace
the unsafe strcpy call in the no-new-messages branch with a bounded copy using
snprintf or strlcpy, ensuring the translated string is truncated safely and buf
remains null-terminated within its 64-byte capacity.
Source: Linters/SAST tools
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
requires lvgl 9.4.0
Summary by CodeRabbit