search

mohabouje / WinToast

WinToast is a lightly library written in C++ which brings a complete integration of the modern toast notifications of Windows 8 & Windows 10. Toast notifications allows your app to inform the users about relevant information and timely events that they should see and take action upon inside your app, such as a new instant message, a new friend request, breaking news, or a calendar event.
MIT License
688 stars 127 forks source link

Fix memory leak in showToast #47

Closed batortaller closed 2 years ago

batortaller commented 4 years ago

showToast only frees the handler when there is no error. This pull request fixes it by using an std::unique_ptr as an input parameter for the handler so that it will be freed in each case.

ltjax commented 4 years ago

I like this! Before, there was not really a way to prevent a leak in case of an error, especially since one error path took ownership of the pointer, while all the others did not. However, with this approach, you should probably std::move instead of .release() to avoid subtle leaks between .release() and the shared_ptr<> c'tor. But then again, why not use a shared_ptr<> right away?

lighterowl commented 4 years ago

You should use the shared_ptr(unique_ptr&&) constructor instead of calling unique_ptr::release(), as doing so will also transfer the unique_ptr's deleter to the shared_ptr, which would be expected when calling such an interface.

lighterowl commented 4 years ago

Actually, scratch that : it actually makes more sense to have a shared_ptr here, as the caller can then have a handler object per notification without having an external mapping via the returned notification ID.