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

UICommon/ResourcePack: Minor cleanup #8127

Merged
merged 6 commits into from May 27, 2019

Conversation

@lioncash
Copy link
Member

lioncash commented May 27, 2019

Just performs some basic cleanups. Notably, the use of scope guards avoids resource leaks due to the file opened with unzOpen not having unzClose called on it in the failure paths. Also removes an unused variable in the closely related manager class.

lioncash added 6 commits May 27, 2019
Same behavior, only it doesn't unnecessarily store a pointer in the
executable. While we're at it, make it constexpr and move it into the
namespace.
We can simply construct the containers with the desired size in these
cases.
A few cases duplicate the string patch creation, which is kind of
wasteful. We can just construct the string once.
Makes it way harder to introduce resource leaks, and plugs the existing
resource leaks in the constructor and Install() where the file wouldn't
be closed in some error cases.
…any contiguous container

This allows the same code to be used to read into a std::string, which
allows for eliminating the vector->string transfer when reading the
manifest file.

A ContiguousContainer is a concept that includes std::array,
std::string, and std::vector.

manifest_contents.resize(manifest_info.uncompressed_size);

std::string manifest_contents(manifest_info.uncompressed_size, '\0');
if (!ReadCurrentFileUnlimited(file, manifest_contents))

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN May 27, 2019

Member

Does this one handle a string parameter just fine?

This comment has been minimized.

Copy link
@lioncash

lioncash May 27, 2019

Author Member

It does indeed. It's the basis for the last commit in the PR.

@leoetlino leoetlino merged commit 800d875 into dolphin-emu:master May 27, 2019
10 checks passed
10 checks passed
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@lioncash lioncash deleted the lioncash:resource branch May 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.