Feature/btree#13
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds FileArchive and GenericFileManager I/O APIs, introduces BehaviorTree and RenderThread runtime modules, adds an AssetBaker CLI stub, and updates build wiring plus MIT license headers across source files. ChangesNew Modules and API Expansion
MIT License Header Additions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/tools/assetbaker/main.cpp (3)
70-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOutput filesize prints the wrong field.
Line 70 prints
stats.inputSizeinstead ofstats.outputSize.Proposed fix
- std::cout << "Output filesize: " << stats.inputSize << std::endl; + std::cout << "Output filesize: " << stats.outputSize << std::endl;🤖 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 `@src/tools/assetbaker/main.cpp` at line 70, In the std::cout statement that prints "Output filesize:", the code is currently printing the wrong statistics field. Change the output statement to use stats.outputSize instead of stats.inputSize, since the message indicates it should display the output file size, not the input file size.
54-59: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
readManifestignores failure and always reports success.Line 57 result is never validated, and Line 58 always returns
true. This allows missing/invalid input to continue as successful processing.Proposed fix
bool readManifest(const std::string& input, MemoryStatistics& stats) { std::cout << "Try to read input manifest " << input << std::endl; GenericFileManager fm; - auto file = fm.createFileReader(input.c_str()); - return true; + FileArchive *file = fm.createFileReader(input.c_str()); + if (file == nullptr || !file->isValid()) { + return false; + } + stats.inputSize = file->getSize(); + fm.close(file); + return true; }🤖 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 `@src/tools/assetbaker/main.cpp` around lines 54 - 59, The readManifest function always returns true without validating whether the file was actually opened successfully. The result of fm.createFileReader(input.c_str()) assigned to the file variable is never checked for validity. Add a validation check to ensure the file object was created successfully before returning true, and return false if the file creation fails, so that invalid or missing input files are properly reported as failures.
37-71: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove helper symbols into the project namespace hierarchy.
MemoryStatisticsand helper functions are currently global. Keepmainglobal, but place supporting symbols under the approvedsegfaultmodule namespace.As per coding guidelines, “All code must live in the
segfaultnamespace with sub-namespaces for modules:segfault::core,segfault::renderer,segfault::application.”🤖 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 `@src/tools/assetbaker/main.cpp` around lines 37 - 71, The MemoryStatistics struct and helper functions (getVersion, showHelp, readManifest, writeAssetArchive, showStatistics) are currently defined in the global namespace, but according to project guidelines they must be placed under the segfault namespace hierarchy. Wrap the struct and all these helper functions with the appropriate segfault module namespace (such as segfault::application or a module-specific namespace for assetbaker), while keeping the main function at the global scope. This ensures all supporting code adheres to the namespace hierarchy without affecting the entry point.Source: Coding guidelines
src/runtime/core/filearchive.h (2)
115-117: 🩺 Stability & Availability | 🔴 CriticalGuard destructor close on invalid stream.
Line 116 unconditionally calls
fclose(mStream). Iffopenfails at line 105 or ifmStreamis passed as null to the second constructor (line 50),fclose(nullptr)triggers undefined behavior. The membermStreamis initialized tonullptr(line 94) and can remain null, but the destructor has no null check despiteisValid()(line 65) checking for this condition.Proposed fix
inline FileArchive::~FileArchive() { - fclose(mStream); + if (mStream != nullptr) { + fclose(mStream); + } }🤖 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 `@src/runtime/core/filearchive.h` around lines 115 - 117, The destructor ~FileArchive() unconditionally calls fclose(mStream) without checking if mStream is null, which causes undefined behavior when mStream was never successfully opened or initialized. Add a null check before calling fclose(mStream) in the destructor body, similar to the pattern used in the isValid() method, to guard against the cases where mStream remains nullptr due to failed fopen calls or null values passed to the second constructor.
143-145: 🎯 Functional Correctness | 🟠 Major
write()returns element count (0 or 1), not bytes written.The Doxygen documentation states "
@returnThe number of bytes actually written", but line 144'sfwrite(buffer, size, 1, mStream)returns the count of size-byte elements written—which is always 0 or 1, not the byte count. Theread()method (line 140) correctly usesfread(buffer, 1, size, mStream)to return bytes. Swap the parameters to match:Fix
inline size_t FileArchive::write(const uint8_t *buffer, size_t size) { - return fwrite(buffer, size, 1, mStream); + return fwrite(buffer, 1, size, mStream); }🤖 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 `@src/runtime/core/filearchive.h` around lines 143 - 145, The write() method in the FileArchive class has incorrect fwrite() parameters. The current call fwrite(buffer, size, 1, mStream) returns the count of size-byte elements written (0 or 1) instead of the number of bytes written. To match the read() method which correctly uses fread(buffer, 1, size, mStream), swap the second and third parameters in the fwrite() call so that it writes 1 byte at a time for size iterations, making the return value consistent with the documented byte count return value.src/runtime/renderer/renderthread.cpp (1)
42-56: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
run()cannot terminate;stop()/waitForCompletion()may hang forever.
isRunning()is derived frommThread.joinable(), andjoinable()stays true while the thread is alive. That makeswhile (isRunning())non-terminating, sojoin()can block indefinitely (including app shutdown paths that ownRenderThread).🐛 Proposed fix
diff --git a/src/runtime/renderer/renderthread.h b/src/runtime/renderer/renderthread.h @@ `#include` "core/segfault.h" +#include <atomic> `#include` <thread> @@ private: std::thread mThread; + std::atomic_bool mRunning{ false }; };diff --git a/src/runtime/renderer/renderthread.cpp b/src/runtime/renderer/renderthread.cpp @@ void RenderThread::start() { - if (!mThread.joinable()) { + if (!mThread.joinable()) { + mRunning.store(true, std::memory_order_release); mThread = std::thread(&RenderThread::run, this); } } @@ void RenderThread::stop() { + mRunning.store(false, std::memory_order_release); if (mThread.joinable()) { mThread.join(); } } @@ bool RenderThread::isRunning() const { - return mThread.joinable(); + return mRunning.load(std::memory_order_acquire); } @@ void RenderThread::run() { - while (isRunning()) { + while (mRunning.load(std::memory_order_acquire)) { // Perform rendering tasks + std::this_thread::yield(); } } @@ void RenderThread::waitForCompletion() { - if (mThread.joinable()) { - mThread.join(); - } + stop(); }Also applies to: 60-63
🤖 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 `@src/runtime/renderer/renderthread.cpp` around lines 42 - 56, The `run()` method has a non-terminating loop because `isRunning()` checks `mThread.joinable()` which remains true while the thread is alive, preventing proper termination. Add a separate atomic boolean flag (e.g., `mShouldRun`) to the RenderThread class to control thread termination. In the `stop()` method, set this flag to false before calling `mThread.join()`. Modify the `isRunning()` method to check this atomic flag instead of `mThread.joinable()`, and use the same flag in the `while (isRunning())` condition in the `run()` method. This allows the thread to exit gracefully when `stop()` is called, preventing indefinite blocking on `join()`.
🧹 Nitpick comments (1)
src/runtime/renderer/renderthread.h (1)
31-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Doxygen API comments to
RenderThreaddeclarations.This header introduces a public interface without
@brief/@returndocumentation on the class and methods, which breaks the header documentation standard.As per coding guidelines,
**/*.h: "Use Doxygen-style comments in headers with@brief,@param, and@returntags".🤖 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 `@src/runtime/renderer/renderthread.h` around lines 31 - 47, Add Doxygen-style documentation comments to the RenderThread class and all its public methods. Place a comment block above the RenderThread class declaration with `@brief` describing the class purpose. For each method (start, stop, isRunning, run, waitForCompletion, join, detach, isJoinable, isDetached, setThreadName, getThreadName, getThreadId), add a comment block with `@brief` describing what it does, `@param` tags for any parameters, and `@return` tags describing the return value or void if applicable. Follow the Doxygen format standard used in your codebase.Source: Coding guidelines
🤖 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 `@src/examples/hello_world/hello_world.cpp`:
- Around line 1-22: Update the copyright notice line in the MIT license header
comment to match the canonical LICENSE file. Replace the line reading "Copyright
(c) 2015-2026 Segfault by Kim Kulling" with "Copyright (c) 2025 Kim Kulling".
This change needs to be applied to all 8 files that have MIT headers in this PR,
including hello_world.cpp, to ensure consistency and proper license compliance.
In `@src/runtime/ai/behavior_tree.cpp`:
- Around line 58-67: The init() method in the BehaviorTree class returns true
without actually creating and assigning the mRootNode member variable. This
causes update() to always return early at the nullptr check, making the tree
non-functional despite reporting successful initialization. Modify the init()
method to create and assign the root node before returning, and only return true
if the root node is successfully created; return false if initialization fails
to prevent false positive initialization reports.
- Around line 53-57: The JSON parsing in the init() function uses a throwing
parse method that can crash on malformed input, and the code doesn't validate
that the archive.read() operation completed successfully. Additionally, the
parsed JSON result is never assigned to mRootNode, causing the function to
return true even when tree initialization fails. Fix this by: wrapping the
archive.read() call to verify that the number of bytes read equals the expected
size, using non-throwing JSON parse with proper error handling to catch parse
failures, and assigning the successfully parsed JSON to mRootNode before
returning true. Return false if the read is incomplete or parsing fails.
In `@src/runtime/ai/behavior_tree.h`:
- Line 27: The namespace declaration `segfault::ai` in the behavior_tree.h file
violates the project's namespace contract, which only allows `segfault::core`,
`segfault::renderer`, and `segfault::application`. Replace all occurrences of
the `segfault::ai` namespace (including the one at line 27 and the additional
occurrence mentioned at line 119) with the appropriate compliant namespace from
the allowed list that best fits the module's purpose.
In `@src/runtime/core/filearchive.h`:
- Around line 119-128: The getSize() method lacks error checking for ftell() and
fseek() operations, which can return error indicators (-1 for ftell, non-zero
for fseek) that when cast to size_t produce incorrect large values. Add
validation after each ftell() call to check if the return value is -1, and after
each fseek() call to check if the return value is non-zero, returning 0
immediately if any error is detected. Follow the same error checking pattern
already correctly implemented in the seek() method (referenced at line 136) to
ensure consistent error handling throughout the FileArchive class.
---
Outside diff comments:
In `@src/runtime/core/filearchive.h`:
- Around line 115-117: The destructor ~FileArchive() unconditionally calls
fclose(mStream) without checking if mStream is null, which causes undefined
behavior when mStream was never successfully opened or initialized. Add a null
check before calling fclose(mStream) in the destructor body, similar to the
pattern used in the isValid() method, to guard against the cases where mStream
remains nullptr due to failed fopen calls or null values passed to the second
constructor.
- Around line 143-145: The write() method in the FileArchive class has incorrect
fwrite() parameters. The current call fwrite(buffer, size, 1, mStream) returns
the count of size-byte elements written (0 or 1) instead of the number of bytes
written. To match the read() method which correctly uses fread(buffer, 1, size,
mStream), swap the second and third parameters in the fwrite() call so that it
writes 1 byte at a time for size iterations, making the return value consistent
with the documented byte count return value.
In `@src/runtime/renderer/renderthread.cpp`:
- Around line 42-56: The `run()` method has a non-terminating loop because
`isRunning()` checks `mThread.joinable()` which remains true while the thread is
alive, preventing proper termination. Add a separate atomic boolean flag (e.g.,
`mShouldRun`) to the RenderThread class to control thread termination. In the
`stop()` method, set this flag to false before calling `mThread.join()`. Modify
the `isRunning()` method to check this atomic flag instead of
`mThread.joinable()`, and use the same flag in the `while (isRunning())`
condition in the `run()` method. This allows the thread to exit gracefully when
`stop()` is called, preventing indefinite blocking on `join()`.
In `@src/tools/assetbaker/main.cpp`:
- Line 70: In the std::cout statement that prints "Output filesize:", the code
is currently printing the wrong statistics field. Change the output statement to
use stats.outputSize instead of stats.inputSize, since the message indicates it
should display the output file size, not the input file size.
- Around line 54-59: The readManifest function always returns true without
validating whether the file was actually opened successfully. The result of
fm.createFileReader(input.c_str()) assigned to the file variable is never
checked for validity. Add a validation check to ensure the file object was
created successfully before returning true, and return false if the file
creation fails, so that invalid or missing input files are properly reported as
failures.
- Around line 37-71: The MemoryStatistics struct and helper functions
(getVersion, showHelp, readManifest, writeAssetArchive, showStatistics) are
currently defined in the global namespace, but according to project guidelines
they must be placed under the segfault namespace hierarchy. Wrap the struct and
all these helper functions with the appropriate segfault module namespace (such
as segfault::application or a module-specific namespace for assetbaker), while
keeping the main function at the global scope. This ensures all supporting code
adheres to the namespace hierarchy without affecting the entry point.
---
Nitpick comments:
In `@src/runtime/renderer/renderthread.h`:
- Around line 31-47: Add Doxygen-style documentation comments to the
RenderThread class and all its public methods. Place a comment block above the
RenderThread class declaration with `@brief` describing the class purpose. For
each method (start, stop, isRunning, run, waitForCompletion, join, detach,
isJoinable, isDetached, setThreadName, getThreadName, getThreadId), add a
comment block with `@brief` describing what it does, `@param` tags for any
parameters, and `@return` tags describing the return value or void if applicable.
Follow the Doxygen format standard used in your codebase.
🪄 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
Run ID: 1312791a-2035-41d7-be2e-694fe3c5fd5f
📒 Files selected for processing (30)
src/examples/hello_world/hello_world.cppsrc/runtime/CMakeLists.txtsrc/runtime/ai/.behavior_tree.cpp.un~src/runtime/ai/.behavior_tree.h.un~src/runtime/ai/behavior_tree.cppsrc/runtime/ai/behavior_tree.hsrc/runtime/application/app.cppsrc/runtime/application/app.hsrc/runtime/core/argumentparser.cppsrc/runtime/core/argumentparser.hsrc/runtime/core/filearchive.hsrc/runtime/core/genericfilemanager.cppsrc/runtime/core/genericfilemanager.hsrc/runtime/core/ifilemanager.hsrc/runtime/core/segfault.hsrc/runtime/core/segfaultexception.hsrc/runtime/core/tokenizer.cppsrc/runtime/core/tokenizer.hsrc/runtime/renderer/RHI.hsrc/runtime/renderer/RHIVulkan.cppsrc/runtime/renderer/rendercore.hsrc/runtime/renderer/renderthread.cppsrc/runtime/renderer/renderthread.hsrc/runtime/renderer/vulkanbuffer.cppsrc/runtime/renderer/vulkanbuffer.hsrc/runtime/renderer/vulkandevice.cppsrc/runtime/renderer/vulkandevice.hsrc/runtime/renderer/vulkanutils.cppsrc/runtime/renderer/vulkanutils.hsrc/tools/assetbaker/main.cpp
Comment on lines
+1
to
+22
| /*----------------------------------------------------------------------------------------------- | ||
| The MIT License (MIT) | ||
|
|
||
| Copyright (c) 2015-2026 Segfault by Kim Kulling | ||
|
|
||
| Permission is hereby granted, free of charge, to any person obtaining a copy of | ||
| this software and associated documentation files (the "Software"), to deal in | ||
| the Software without restriction, including without limitation the rights to | ||
| use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of | ||
| the Software, and to permit persons to whom the Software is furnished to do so, | ||
| subject to the following conditions: | ||
|
|
||
| The above copyright notice and this permission notice shall be included in all | ||
| copies or substantial portions of the Software. | ||
|
|
||
| THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS | ||
| FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR | ||
| COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER | ||
| IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN | ||
| CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
| -----------------------------------------------------------------------------------------------*/ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Copyright notice in source files does not match the canonical LICENSE file.
The copyright notice should match what is specified in the LICENSE file as the copyright holder. This file (and all others with MIT headers) uses "Copyright (c) 2015-2026 Segfault by Kim Kulling", but the repository's LICENSE file specifies "Copyright (c) 2025 Kim Kulling".
These discrepancies create ambiguity about the correct copyright holder and years:
- Year range: LICENSE says 2025; headers say 2015-2026
- Attribution: LICENSE says "Kim Kulling"; headers say "Segfault by Kim Kulling"
The MIT license requires that "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software", which means copyright notices must be consistent across the LICENSE file and source files for compliance.
This issue affects all 8 files receiving MIT headers in this PR. Update all headers to match the canonical LICENSE file: Copyright (c) 2025 Kim Kulling.
🤖 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 `@src/examples/hello_world/hello_world.cpp` around lines 1 - 22, Update the
copyright notice line in the MIT license header comment to match the canonical
LICENSE file. Replace the line reading "Copyright (c) 2015-2026 Segfault by Kim
Kulling" with "Copyright (c) 2025 Kim Kulling". This change needs to be applied
to all 8 files that have MIT headers in this PR, including hello_world.cpp, to
ensure consistency and proper license compliance.
Comment on lines
+119
to
+128
| inline size_t FileArchive::getSize() const { | ||
| if (!isValid()) { | ||
| return 0; | ||
| } | ||
| long currentPos = ftell(mStream); | ||
| fseek(mStream, 0, SEEK_END); | ||
| long size = ftell(mStream); | ||
| fseek(mStream, currentPos, SEEK_SET); | ||
|
|
||
| return static_cast<size_t>(size); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "size_t FileArchive::getSize|ftell\\(|fseek\\(" src/runtime/core/filearchive.h src/runtime/ai/behavior_tree.cppRepository: kimkulling/Segfault
Length of output: 593
🏁 Script executed:
wc -l src/runtime/core/filearchive.hRepository: kimkulling/Segfault
Length of output: 192
🏁 Script executed:
sed -n '115,135p' src/runtime/core/filearchive.hRepository: kimkulling/Segfault
Length of output: 580
🏁 Script executed:
# Check if there are other ftell/fseek patterns in the codebase without error checking
rg -A 3 "ftell\(" src/ --type cpp --type h | head -40Repository: kimkulling/Segfault
Length of output: 513
🏁 Script executed:
# Verify the isValid() implementation to understand what it checks
ast-grep outline src/runtime/core/filearchive.hRepository: kimkulling/Segfault
Length of output: 202
🏁 Script executed:
# Get the complete file structure with ast-grep
ast-grep outline src/runtime/core/filearchive.h --view expandedRepository: kimkulling/Segfault
Length of output: 202
🏁 Script executed:
# Search for isValid method definition in the file
rg -B 2 -A 2 "isValid()" src/runtime/core/filearchive.hRepository: kimkulling/Segfault
Length of output: 617
🏁 Script executed:
# Search for other ftell/fseek patterns in the entire codebase that might have similar issues
rg -B 2 -A 2 "ftell|fseek" src/ --type cpp --type hRepository: kimkulling/Segfault
Length of output: 806
🏁 Script executed:
# Check the complete FileArchive class header to see all methods
cat -n src/runtime/core/filearchive.hRepository: kimkulling/Segfault
Length of output: 6677
Handle ftell/fseek failures before size conversion.
Lines 123–126 lack error checking. Both ftell() returns -1 on error and fseek() returns non-zero on error. If either fails, line 128 casts a negative value to size_t, yielding a huge unsigned value that poisons downstream allocations. The seek() method correctly validates fseek() (line 136); apply the same pattern here.
Proposed fix
inline size_t FileArchive::getSize() const {
if (!isValid()) {
return 0;
}
- long currentPos = ftell(mStream);
- fseek(mStream, 0, SEEK_END);
- long size = ftell(mStream);
- fseek(mStream, currentPos, SEEK_SET);
+ long currentPos = ftell(mStream);
+ if (currentPos < 0) {
+ return 0;
+ }
+ if (fseek(mStream, 0, SEEK_END) != 0) {
+ return 0;
+ }
+ long size = ftell(mStream);
+ if (size < 0) {
+ (void)fseek(mStream, currentPos, SEEK_SET);
+ return 0;
+ }
+ (void)fseek(mStream, currentPos, SEEK_SET);
return static_cast<size_t>(size);
}📝 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
| inline size_t FileArchive::getSize() const { | |
| if (!isValid()) { | |
| return 0; | |
| } | |
| long currentPos = ftell(mStream); | |
| fseek(mStream, 0, SEEK_END); | |
| long size = ftell(mStream); | |
| fseek(mStream, currentPos, SEEK_SET); | |
| return static_cast<size_t>(size); | |
| inline size_t FileArchive::getSize() const { | |
| if (!isValid()) { | |
| return 0; | |
| } | |
| long currentPos = ftell(mStream); | |
| if (currentPos < 0) { | |
| return 0; | |
| } | |
| if (fseek(mStream, 0, SEEK_END) != 0) { | |
| return 0; | |
| } | |
| long size = ftell(mStream); | |
| if (size < 0) { | |
| (void)fseek(mStream, currentPos, SEEK_SET); | |
| return 0; | |
| } | |
| (void)fseek(mStream, currentPos, SEEK_SET); | |
| return static_cast<size_t>(size); | |
| } |
🤖 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 `@src/runtime/core/filearchive.h` around lines 119 - 128, The getSize() method
lacks error checking for ftell() and fseek() operations, which can return error
indicators (-1 for ftell, non-zero for fseek) that when cast to size_t produce
incorrect large values. Add validation after each ftell() call to check if the
return value is -1, and after each fseek() call to check if the return value is
non-zero, returning 0 immediately if any error is detected. Follow the same
error checking pattern already correctly implemented in the seek() method
(referenced at line 136) to ensure consistent error handling throughout the
FileArchive class.
Added new namespaces for AI and scene code.
Add namespaces for AI and scene in AGENTS.md
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Documentation