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

Fixed potential memory leak in sf::Font #509

Merged
merged 1 commit into from Feb 15, 2014

Conversation

Projects
None yet
3 participants
@dermoumi
Contributor

dermoumi commented Dec 15, 2013

Nothing much, might have missed something though.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Dec 15, 2013

Member

FT_DoneFace is not needed, it is done in the destructor.
delete rec however is correct.

Member

LaurentGomila commented Dec 15, 2013

FT_DoneFace is not needed, it is done in the destructor.
delete rec however is correct.

@dermoumi

This comment has been minimized.

Show comment
Hide comment
@dermoumi

dermoumi Dec 15, 2013

Contributor

FT_Done_Face IS needed if i see correctly...

FT_Face face; //! The face opened, but not stored in the class
if (FT_Open_Face(static_cast<FT_Library>(m_library), &args, 0, &face) != 0)
{
    err() << "Failed to load font from stream (failed to create the font face)" << std::endl;
    return false;
}

// ....

m_face = face; //! This is the one free'd in the destructor, which is never stored since the function returns before doing so when failing the FT_Select_Charmap()

FT_Face is C stuff, doesn't have destructors (and it seems it's a typedef to a pointer type to top it off)
might be wrong nonetheless, haven't played enough with libfreetype

Contributor

dermoumi commented Dec 15, 2013

FT_Done_Face IS needed if i see correctly...

FT_Face face; //! The face opened, but not stored in the class
if (FT_Open_Face(static_cast<FT_Library>(m_library), &args, 0, &face) != 0)
{
    err() << "Failed to load font from stream (failed to create the font face)" << std::endl;
    return false;
}

// ....

m_face = face; //! This is the one free'd in the destructor, which is never stored since the function returns before doing so when failing the FT_Select_Charmap()

FT_Face is C stuff, doesn't have destructors (and it seems it's a typedef to a pointer type to top it off)
might be wrong nonetheless, haven't played enough with libfreetype

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Dec 16, 2013

Member

Sorry, you're right. I thought you were releasing the m_face member, not the local one.

Member

LaurentGomila commented Dec 16, 2013

Sorry, you're right. I thought you were releasing the m_face member, not the local one.

@ghost ghost assigned LaurentGomila Jan 28, 2014

@Bromeon Bromeon assigned Bromeon and unassigned LaurentGomila Feb 9, 2014

@Bromeon Bromeon added bug labels Feb 12, 2014

Bromeon added a commit that referenced this pull request Feb 15, 2014

Merge pull request #509 from ophui-/master
Fixed potential memory leak in sf::Font

@Bromeon Bromeon merged commit 5266133 into SFML:master Feb 15, 2014

@LaurentGomila LaurentGomila added resolved and removed resolved labels Feb 15, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment