fix leak of DisplayList storage #28158
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| if (ptr_ == other.ptr_) { | ||
| uint8_t* ptr = storage_.get(); | ||
| uint8_t* o_ptr = other.storage_.get(); | ||
| if (ptr == o_ptr) { |
There was a problem hiding this comment.
I believe you should be able to compare the unique_ptrs directly.
if (storage_ == other.storage_) {
...
}
There was a problem hiding this comment.
We need them anyway for the next step.
|
The tests should be taken care of when we add leak detection to the unit tests. |
flar
added
the
waiting for tree to go green
Switch to a unique_ptr to ensure that the memory is freed. The type used here for DisplayList::storage_ here is taken from the internal memory holder of the SkAutoTMalloc that was used to allocate and grow the array. SkAutoTMalloc manages a pointer that should be freed with sk_free (which is just a null protected wrapper around free).