-
Notifications
You must be signed in to change notification settings - Fork 28
Fix double freed in remove_self_from_waiters() #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Can you revisit codebase to eliminate the similar misuse? |
Hi, Jserv, There is no remaining similar misuse after this PR merged. This PR is the last misuse case. |
|
I defer to @HeatCrab for confirmation. |
HeatCrab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I've reviewed the entire codebase for similar misuse patterns and confirmed that this is the only remaining instance that needs to be fixed.
LGTM.
jserv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Append Close #57 at the end of git commit message.
free() has been called in list_remove(). This commit removes free to prevent doubly function call. close sysprog21#57
697fd63 to
4948444
Compare
visitorckw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code change itself LGTM.
I'm not sure if this description was generated by an LLM, but "prevent doubly function call' sounds a bit strange to me. Calling a function twice isn't an issue, whereas calling free() twice on the same pointer definitely is.
Also, did you omit the user impact from the commit message because it's already described in the issue? I think it's short enough to be included here. The commit message should be self-contained without requiring the reader to check external links.
One last thing: please avoid starting with "This commit". We know it's a commit. Please use the imperative mood instead.
free() has been called in list_remove().
This commit removes free to prevent doubly function call.
close #57
Summary by cubic
Removed redundant free in remove_self_from_waiters() to prevent a double-free when removing a waiter. list_remove() already frees the node, avoiding crashes and memory corruption in the mutex waiters list.
Written for commit 4948444. Summary will update automatically on new commits.