Skip to content

Commit

Permalink
Fix memory leak
Browse files Browse the repository at this point in the history 
If the result is successful, then the allocated memory is not cleared. This adds deallocate memory if the result is successful.
  • Loading branch information
@JarosLucky @NeilJed
JarosLucky authored and NeilJed committed Nov 25, 2019
1 parent cb173a8 commit f5e7991
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions VTFLib/VTFFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,17 @@ vlBool CVTFFile::Create(vlUInt uiWidth, vlUInt uiHeight, vlUInt uiFrames, vlUInt
this->SetStartFrame(VTFCreateOptions.uiStartFrame);
this->SetBumpmapScale(VTFCreateOptions.sBumpScale);

if(lpNewImageDataRGBA8888 != 0)
{
for(vlUInt i = 0; i < uiCount; i++)
{
delete []lpNewImageDataRGBA8888[i];
}
delete []lpNewImageDataRGBA8888;
}

this->Destroy();

return vlTrue;
}
catch(...)
Expand Down

5 comments on commit f5e7991

@misyltoad
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. Clearly nobody tested this before it was merged.

@JarosLucky
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Joshua-Ashton One might ask. Wrong about what? That the co-creation occurred without checking on the project, which in this situation can not be compiled, or the solution itself is wrong?

@JarosLucky
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, in reviewing this code, I only see a wrong call to this->Destroy(); when the change is successful.

@misyltoad
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the issue.

@JarosLucky
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the issue.

Sorry, but what do you mean? or what does it refer to?

Please sign in to comment.