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

Fix Miscellaneous Regressions and UBs #1022

Merged
merged 9 commits into from May 29, 2019

Conversation

@Headline
Copy link
Member

Headline commented May 27, 2019

Ran the cppcheck static analyzer against /core/ to check if we were doing any ugliness that I didn't know about. Turns out there's a few situations where we were regressed.

First, and likely the most significant, was CExtension::FindNextDependency. This method must not have worked correctly for many many years. All operations were mistakenly done on a default-constructed iterator.

Secondly, realloc can fail. By doing assignment directly on the result of realloc we miss the opportunity to check if the retval is null or not. If it is null, then our memory allocated previously is still valid and we've just successfully leaked it. For the case involving InternalFormat, an exception was added for realloc failures.

Finally, 1<<31 is undefined behavior as 1 is an int32.

Also switched some casts to static_cast because this is c++, after all...

core/logic/stringutil.cpp Outdated Show resolved Hide resolved
Headline added 2 commits May 27, 2019
@Headline Headline force-pushed the ub-fixes branch from 747ce87 to 22ffcc4 May 28, 2019
@Headline

This comment has been minimized.

Copy link
Member Author

Headline commented May 28, 2019

Since dependency iteration has never historically functioned correctly, it is now deprecated.

Extensions have a very specific place alongside SourceMod, and their goal is to extend SourceMod's plugin API and provide an extension to SourceMod functionality. There was never really a need for this feature in the first place and this change allows us to further solidify our expectations for what an extension is meant to be.

@KyleSanderson KyleSanderson requested a review from dvander May 28, 2019
@Headline Headline added the Bug label May 28, 2019
@Headline Headline merged commit 2803696 into master May 29, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Headline Headline deleted the ub-fixes branch May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.