Skip to content
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

IconHelper leaves unsed API allocated memory #72

Closed
wants to merge 1 commit into from

Conversation

VBWebprofi
Copy link

Your working with the Windows API is dirty. You don't frees API allocated memory. In description of ExtractIconEx states under "Remarks" you have to destroy the icons, that means calling DestroyIcon for each icon handle (your IntPtr's).

I'm not familiar right now with GIT so I don't fix it in code, but introduces a "TODO" comment with links to the MSDN descriptions of both functions.

Your working with the Windows API is dirty. You don't frees API allocated memory. In description of ExtractIconEx states under "Remarks" you have to destroy the icons, that means calling DestroyIcon for each icon handle (your IntPtr's).

I'm not familiar right now with GIT so I don't fix it in code, but introduces a "TODO" comment with links to the MSDN descriptions of both functions.
@Kyrodan
Copy link
Owner

Kyrodan commented Feb 2, 2017

Thanks for your PR. I will check this out.

@Kyrodan
Copy link
Owner

Kyrodan commented Feb 10, 2017

Maybe I will remove this completely:
It has no real additional benefit, is not compatible with Mono and the extracted icons do not look good. I prefer using two static icons: the KeePass-Icon for databases and a generic document icon for all other files.

@Kyrodan Kyrodan added this to the 1.4.0 milestone May 30, 2017
Kyrodan added a commit that referenced this pull request May 30, 2017
@Kyrodan Kyrodan closed this May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants