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

mlx_destroy_display leaves memory unfreed #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

UnWaDo
Copy link

@UnWaDo UnWaDo commented Feb 17, 2022

mlx_destroy_display was added to free memory, allocated with mlx_init. Unfortunately, now it leaves t_xvar *xvar unfreed, so memory is still leaking

`mlx_destroy_display` was added to free memory, allocated with `mlx_init`. Unfortunately, now it leaves `t_xvar *xvar` unfreed, so memory is still leaking
@ggjulio
Copy link
Contributor

ggjulio commented Jun 6, 2022

I kinda agree.
mlx_destroy_display should never have existed/merged because it's not the proper destructor of mlx_init.

Then students break the Soc principle by freeing themselves the object returned by mlx_init. In order to "have 0 leaks".
I know most of studs thinks still reachables reported by valgrind are bad things that must disappear at all cost.
But must of the time it is okay to have some and don't deserve a leak flag.

For example as OL said, (author of mlx)
the still reachable "leak" of mlx_init doesn't matter since all our program's lifetime ends when graphics ends.

However a proper destructor would have been necessary if our program continue after graphics ends.
(to release the memory right when we don't need it anymore)

Also mlx_destroy_display is a complicated name for what it does.
Probably mlx_quit would be better.
Like the SDL -> SDL_Init() and SQL_Quit()

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