-
Notifications
You must be signed in to change notification settings - Fork 108
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
Fix pc_patch_compress leak (take 2) #206
Conversation
And call that function from all the different pc_patch_*_free functions.
pc_patch_free_stats should always be called with a valid patch.
Closes #111. |
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.
LGTM, thanks @elemoine for polishing this !
lib/pc_patch_uncompressed.c
Outdated
@@ -215,6 +216,8 @@ pc_patch_uncompressed_compute_extent(PCPATCH_UNCOMPRESSED *patch) | |||
void | |||
pc_patch_uncompressed_free(PCPATCH_UNCOMPRESSED *patch) | |||
{ | |||
pc_patch_free_stats((PCPATCH*) patch); | |||
|
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.
As you removed the check in pc_patch_free_stats
, maybe you can add assert(patch);
here, just to be consistent across patch types
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.
Good point. I'll add that.
Nice picture by the way ;)
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.
New commit added.
This PR replaces #112.
I've taken @achidlow's commits, squashed them together, and added three additional commits. My second commit renames the pc_patch_common_free function to pc_patch_free_stats, which is more explicit.
@mbredif can you please take a final look at this before I merge it and release 1.1.0?