fixed white space invalid free #6
Conversation
WalkthroughThe pull request updates the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/core/parser.c (4)
25-25: Variable naming could be improved for consistency and clarity.The variable name
first_addres_of_input_copyhas a spelling error ("addres" should be "address") and doesn't follow consistent naming conventions. Consider using something likeoriginal_input_copywhich better describes its purpose.- char *first_addres_of_input_copy = input_copy; + char *original_input_copy = input_copy;
29-30: Proper handling of memory after trim_whitespace - good fix.This change correctly ensures we're freeing the original memory address rather than the modified pointer returned by trim_whitespace. Setting the pointer to NULL afterward is also good practice.
- free(first_addres_of_input_copy); - first_addres_of_input_copy =NULL; + free(first_addres_of_input_copy); + first_addres_of_input_copy = NULL;
37-38: Missing space in NULL assignment.There should be a space before NULL in the assignment.
- free(first_addres_of_input_copy); - first_addres_of_input_copy =NULL; + free(first_addres_of_input_copy); + first_addres_of_input_copy = NULL;
91-94: Final memory cleanup - needs spacing improvements.The code correctly ensures the original allocation is freed even in the success path, but has style issues.
- if(first_addres_of_input_copy){ + if (first_addres_of_input_copy) { free(first_addres_of_input_copy); first_addres_of_input_copy = NULL; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/parser.c(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/core/parser.c (1)
src/utils/helpers.c (1) (1)
trim_whitespace(14:30)
🔇 Additional comments (2)
src/core/parser.c (2)
46-48: Memory cleanup in error path - good fix.This correctly frees the original memory allocation when args allocation fails, preventing a memory leak.
25-94: This is a correct fix for the invalid free pointer issue.The original bug occurred because
trim_whitespace()modifies the pointer in-place when removing leading whitespace, causing the original allocation address to be lost. By storing the original pointer infirst_addres_of_input_copy, you ensure the correct address is used when freeing memory.This change effectively addresses the issue described in the PR where commands without a space were causing invalid free pointers.
|
@mateusmoutinho everything LGTM. Can you address the reviews coderabbit has added. |
mateusmoutinho
left a comment
mateusmoutinho
left a comment
There was a problem hiding this comment.
for me codde rabbit didint changed anything of I did
2 participants
if I executed the app and pass ** ( ccomand) **, it generates a free invalid pointer
yes , you must pass [SPACE,comand]
example:
./nutshell ---> lsExplanation:
on the file: src/core/parser.c
on the function: **ParsedCommand parse_command(char input){ (line 16)
in part : ** input_copy = trim_whitespace(input_copy);** (line 26), you were remove a + size of the string
making the oriignal pointer, became some bytes higher than the original size
causing invalid pointer on free
Solution
create a copy of the original pointer
Summary by CodeRabbit