Fix memory leaks in attr_test.c#44
Merged
Merged
Conversation
- Add AddressSanitizer job to detect memory errors (buffer overflow, use-after-free, etc.) - Add UndefinedBehaviorSanitizer job to detect undefined behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Capture return value of attr_add_list() which creates a new combined list when list_of_lists==0, instead of discarding it - Free strings returned by attr_list_to_string() These fixes resolve ASan leak detection failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The loop was passing the parent list 'l' instead of the sublist 'l->l.lists.lists[i]', causing infinite recursion when encoding a list_of_lists structure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
attr_join_lists increments ref counts of sublists, so after freeing the combined list, the original lists still have ref_count=1 and must be freed separately before reassigning the variables. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
set_attr takes ownership of string pointers and free_attr_list frees them. Passing a string literal causes a crash when the list is freed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
add_opaque_attr takes ownership of the buffer and free_attr_list frees it. Passing a stack buffer causes a crash when the list is freed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
Fix memory leaks detected by AddressSanitizer in
attr_test.cand a pre-existing bug inattr.c:Test fixes (attr_test.c)
attr_add_list()which creates a new combined list whenlist_of_lists==0, instead of discarding itattr_list_to_string()Library bug fix (attr.c)
recursive_encode()forlist_of_listsstructureslinstead of the sublistl->l.lists.lists[i]Details
The test changes exposed a latent bug in the library:
attr_add_list()callsattr_join_lists()when the first list doesn't already havelist_of_listsset. This creates a new list containing both inputs. The test was discarding this return value, soattr_list_to_string()was never called on alist_of_listsstructure, hiding the infinite recursion bug.Test plan
🤖 Generated with Claude Code