Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upFullpipe Common::String + memory leak fixes #925
Merged
Conversation
engines/fullpipe/scene.cpp
Outdated
| @@ -221,10 +210,10 @@ bool Scene::load(MfcArchive &file) { | |||
|
|
|||
| _libHandle = g_fp->_currArchive; | |||
|
|
|||
| if (_picObjList.size() > 0 && _bgname && strlen(_bgname) > 1) { | |||
| if (_picObjList.size() > 0 && _bgname.size() > 1) { | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Checking for an empty string is what the original code does, according to sev, so the check for a string of size 1 was wrong
_bitmap is initialized with new, so it needs to be freed with delete, not free()
The TODO in the code in question should be reviewed, but the call to freePixelData() unconditionally deleted the original bitmap, which is not correct
Free the pixel data of each entry in the _dynamicPhases array before emptying it
bluegr
changed the title
|
Sweet. |
|
Looks good, but I don't understand why the Common::String function arguments are a mix of by-value and by-(non-const-)reference, instead of the standard |
|
Good point @wjp. I'll adapt these accordingly on master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
bluegr commentedMar 22, 2017
•
edited
In this pull request, I've changed a lot of engine strings in Fullpipe so that they are handled via Common::String. This results in cleaner and more manageable code.
I've tried splitting up the commits so that all relevant changes are grouped together
Update: I've also added some memory leak fixes to this branch, as part of fixing memory leaks in this engine. These are relevant, because the original intention of this branch was to optimize code and identify memory leaks.