Conversation
FreddyMSchubert
added
client_lib
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @FreddyMSchubert. * #186 (comment) The following files were modified: * `bots/c/client_lib/src/public/debug.c` * `bots/c/hardcore/my-core-bot/src/main.c` * `bots/c/softcore/my-core-bot/src/main.c`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wiki/reference/debug/core_debug_addObjectInfo.md (1)
31-31:⚠️ Potential issue | 🟡 MinorParameters section still references
infoinstead offormat, and...is undocumented.The signature was updated to
(const t_obj *obj, const char *format, ...)but the params block wasn't fully updated.📝 Proposed fix
-- `const char *info`: The string which will be attached to the debug data. +- `const char *format`: A printf-style format string. +- `...`: Format arguments matching the specifiers in `format`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wiki/reference/debug/core_debug_addObjectInfo.md` at line 31, Update the Parameters section for core_debug_addObjectInfo to match the new signature: replace the `const char *info` entry with `const char *format` and add a short line documenting the variadic `...` (e.g., "Additional arguments for the format string, as in printf") and ensure the `const t_obj *obj` parameter is present and described; reference the function name core_debug_addObjectInfo and the parameters `(const t_obj *obj, const char *format, ...)` so the docs accurately reflect the current API.
🧹 Nitpick comments (1)
bots/c/client_lib/src/public/debug.c (1)
75-75: Redundantstrlen(msg)—neededalready has the length.
neededwas computed by the firstvsnprintfcall and equalsstrlen(msg)by construction. Reusing(size_t)neededavoids the redundant traversal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bots/c/client_lib/src/public/debug.c` at line 75, Replace the redundant strlen call: use the already-computed needed value (from the first vsnprintf call) instead of recomputing strlen(msg); specifically, remove or replace the variable new_len = strlen(msg) and set new_len (or use the length where needed) to (size_t)needed so the code uses the previously computed needed value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bots/c/client_lib/inc/core_lib.h`:
- Around line 277-278: The doc comment for core_debug_addObjectInfo is stale:
replace the `@param` info line with proper documentation for the variadic
signature by documenting `@param` format (the printf-style format string) and
`@param` ... (the variadic arguments used with format); update any brief
description to note printf-style formatting and mention that arguments follow
format to match the function prototype core_debug_addObjectInfo(const t_obj
*obj, const char *format, ...).
In `@bots/c/client_lib/src/public/debug.c`:
- Around line 74-78: The current code assigns realloc() directly to entry->info
which can lose the original pointer on failure and cause undefined behavior when
calling strcat(entry->info, msg); change this to use a temporary pointer (e.g.,
char *tmp = realloc(entry->info, old_len + new_len + 1)); check if tmp is NULL
and if so free msg and handle the error (return or set an error code) without
modifying entry->info; on success assign entry->info = tmp and then call
strcat(entry->info, msg) and free(msg). Ensure you reference and update the
existing variables entry->info, msg, old_len, new_len and the calls to realloc
and strcat in this block.
---
Outside diff comments:
In `@wiki/reference/debug/core_debug_addObjectInfo.md`:
- Line 31: Update the Parameters section for core_debug_addObjectInfo to match
the new signature: replace the `const char *info` entry with `const char
*format` and add a short line documenting the variadic `...` (e.g., "Additional
arguments for the format string, as in printf") and ensure the `const t_obj
*obj` parameter is present and described; reference the function name
core_debug_addObjectInfo and the parameters `(const t_obj *obj, const char
*format, ...)` so the docs accurately reflect the current API.
---
Nitpick comments:
In `@bots/c/client_lib/src/public/debug.c`:
- Line 75: Replace the redundant strlen call: use the already-computed needed
value (from the first vsnprintf call) instead of recomputing strlen(msg);
specifically, remove or replace the variable new_len = strlen(msg) and set
new_len (or use the length where needed) to (size_t)needed so the code uses the
previously computed needed value.
| size_t old_len = strlen(entry->info); | ||
| size_t new_len = strlen(info); | ||
| size_t new_len = strlen(msg); | ||
| entry->info = realloc(entry->info, old_len + new_len + 1); | ||
| strcat(entry->info, info); | ||
| strcat(entry->info, msg); | ||
| free(msg); |
There was a problem hiding this comment.
realloc result must not be stored directly into entry->info.
If realloc returns NULL, the original entry->info allocation is lost (memory leak) and the subsequent strcat(NULL, msg) is undefined behavior. Additionally, since msg is a new allocation introduced by this PR, it is also leaked on this failure path.
🐛 Proposed fix
size_t old_len = strlen(entry->info);
- size_t new_len = strlen(msg);
- entry->info = realloc(entry->info, old_len + new_len + 1);
- strcat(entry->info, msg);
- free(msg);
+ size_t new_len = (size_t)needed;
+ char *tmp = realloc(entry->info, old_len + new_len + 1);
+ if (!tmp)
+ {
+ free(msg);
+ return;
+ }
+ entry->info = tmp;
+ memcpy(entry->info + old_len, msg, new_len + 1);
+ free(msg);📝 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.
| size_t old_len = strlen(entry->info); | |
| size_t new_len = strlen(info); | |
| size_t new_len = strlen(msg); | |
| entry->info = realloc(entry->info, old_len + new_len + 1); | |
| strcat(entry->info, info); | |
| strcat(entry->info, msg); | |
| free(msg); | |
| size_t old_len = strlen(entry->info); | |
| size_t new_len = (size_t)needed; | |
| char *tmp = realloc(entry->info, old_len + new_len + 1); | |
| if (!tmp) | |
| { | |
| free(msg); | |
| return; | |
| } | |
| entry->info = tmp; | |
| memcpy(entry->info + old_len, msg, new_len + 1); | |
| free(msg); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bots/c/client_lib/src/public/debug.c` around lines 74 - 78, The current code
assigns realloc() directly to entry->info which can lose the original pointer on
failure and cause undefined behavior when calling strcat(entry->info, msg);
change this to use a temporary pointer (e.g., char *tmp = realloc(entry->info,
old_len + new_len + 1)); check if tmp is NULL and if so free msg and handle the
error (return or set an error code) without modifying entry->info; on success
assign entry->info = tmp and then call strcat(entry->info, msg) and free(msg).
Ensure you reference and update the existing variables entry->info, msg,
old_len, new_len and the calls to realloc and strcat in this block.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bots/c/client_lib/inc/core_lib.h`:
- Line 279: The append path in core_debug_addObjectInfo assigns realloc() result
directly to entry->info and then calls strcat(entry->info, msg), which can
dereference NULL if realloc fails; change the logic to call realloc into a
temporary pointer (e.g. char *new_buf = realloc(entry->info, old_len + new_len +
1)), check new_buf for NULL before updating entry->info, handle the allocation
failure (log via existing debug logger or skip append and still free(msg) to
avoid leaks), and only call strcat on entry->info after the successful realloc;
ensure msg is freed in both success and failure paths.
- Around line 274-279: The documentation for core_debug_addObjectInfo is
misleading about newlines; update the `@details` text in core_lib.h to state that
messages are concatenated without automatic newlines and that callers must
include "\n" in their format strings (matching the debug.c behavior),
referencing the function name core_debug_addObjectInfo so maintainers can find
and keep the comment consistent with the implementation.
| /// @brief Add debug information to an object for visualization | ||
| /// @details This function accumulates debug strings for objects. Multiple calls per tick will append the strings (with newlines). | ||
| /// @param obj The object to attach debug info to | ||
| /// @param info The debug string to add | ||
| void core_debug_addObjectInfo(const t_obj *obj, const char *info); | ||
| /// @param format printf-style format debug string to add | ||
| /// @param ... Format arguments matching the format string | ||
| void core_debug_addObjectInfo(const t_obj *obj, const char *format, ...); |
There was a problem hiding this comment.
@details "(with newlines)" is misleading
The implementation in debug.c concatenates messages with no automatic newline between them — callers must embed \n in their format string (as in the PR example). The parenthetical "(with newlines)" suggests the function does it automatically, which could mislead future users.
📝 Suggested clarification
-/// `@details` This function accumulates debug strings for objects. Multiple calls per tick will append the strings (with newlines).
+/// `@details` This function accumulates debug strings for objects. Multiple calls per tick will append the strings. Include `\n` in the format string to separate entries.📝 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.
| /// @brief Add debug information to an object for visualization | |
| /// @details This function accumulates debug strings for objects. Multiple calls per tick will append the strings (with newlines). | |
| /// @param obj The object to attach debug info to | |
| /// @param info The debug string to add | |
| void core_debug_addObjectInfo(const t_obj *obj, const char *info); | |
| /// @param format printf-style format debug string to add | |
| /// @param ... Format arguments matching the format string | |
| void core_debug_addObjectInfo(const t_obj *obj, const char *format, ...); | |
| /// `@brief` Add debug information to an object for visualization | |
| /// `@details` This function accumulates debug strings for objects. Multiple calls per tick will append the strings. Include `\n` in the format string to separate entries. | |
| /// `@param` obj The object to attach debug info to | |
| /// `@param` format printf-style format debug string to add | |
| /// `@param` ... Format arguments matching the format string | |
| void core_debug_addObjectInfo(const t_obj *obj, const char *format, ...); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bots/c/client_lib/inc/core_lib.h` around lines 274 - 279, The documentation
for core_debug_addObjectInfo is misleading about newlines; update the `@details`
text in core_lib.h to state that messages are concatenated without automatic
newlines and that callers must include "\n" in their format strings (matching
the debug.c behavior), referencing the function name core_debug_addObjectInfo so
maintainers can find and keep the comment consistent with the implementation.
| void core_debug_addObjectInfo(const t_obj *obj, const char *info); | ||
| /// @param format printf-style format debug string to add | ||
| /// @param ... Format arguments matching the format string | ||
| void core_debug_addObjectInfo(const t_obj *obj, const char *format, ...); |
There was a problem hiding this comment.
realloc null-check missing in the implementation — potential crash
In bots/c/client_lib/src/public/debug.c (lines 70–75), the append path overwrites entry->info with the return value of realloc before checking for NULL:
entry->info = realloc(entry->info, old_len + new_len + 1);
strcat(entry->info, msg); // UB / crash if realloc returned NULL
free(msg);If realloc fails it returns NULL, the original entry->info pointer is lost (memory leak), and the subsequent strcat(NULL, msg) is undefined behaviour.
🐛 Proposed fix
- entry->info = realloc(entry->info, old_len + new_len + 1);
- strcat(entry->info, msg);
- free(msg);
+ char *tmp = realloc(entry->info, old_len + new_len + 1);
+ if (!tmp)
+ {
+ free(msg);
+ return;
+ }
+ entry->info = tmp;
+ strcat(entry->info, msg);
+ free(msg);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bots/c/client_lib/inc/core_lib.h` at line 279, The append path in
core_debug_addObjectInfo assigns realloc() result directly to entry->info and
then calls strcat(entry->info, msg), which can dereference NULL if realloc
fails; change the logic to call realloc into a temporary pointer (e.g. char
*new_buf = realloc(entry->info, old_len + new_len + 1)), check new_buf for NULL
before updating entry->info, handle the allocation failure (log via existing
debug logger or skip append and still free(msg) to avoid leaks), and only call
strcat on entry->info after the successful realloc; ensure msg is freed in both
success and failure paths.
Reptudn
left a comment
Reptudn
left a comment
There was a problem hiding this comment.
looking good.. will implement that if it exists in go (it probably does) once the lib is functional
3 participants
Previously you had to manually format a string together with sprintf if you wanted dynamic data in the debug data. This is no longer the case, as you can now use the function like printf. This will hopefully make it so easy to use that more than one person does so. It's also cool and I didn't know it was possible.
core_debug_addObjectInfo(units[i], "I am a warrior! 🗡️ - I am heading for the opponent core at [%d,%d]! 🏰\n", ft_get_core_opponent()->pos.x, ft_get_core_opponent()->pos.y);@Peu77 @Reptudn Tagging you guys for PRs related to the client lib from now on as you guys are writing your own versions so you know when you have to make changes to maintain your version, please do that. Also please review this PR. I don't think tsConn needs this, but I think goConn should also mirror
fmt.Printfonce it's maintained and up-to-date.Summary by CodeRabbit
New Features
Documentation