Cleanup static analyzer issues #179
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kcgen
added a commit
to dosbox-staging/dosbox-staging
that referenced
this pull request
Aug 23, 2021
Fixes PVS Studio issue: V730 Not all members of a class are initialized inside the constructor. Consider inspecting: _verbosity, _line, _name.
Fixes PVS Studio flagged issue: V769 The 'file_path' pointer in the 'file_path + 1' expression could be nullptr. In such case, resulting value will be senseless and it should not be used.
The check to see if info.dli_sname is 0 will always be false because of the prior if-statement requiring it to be non-zero: "if (dladdr(callstack[i], &info) && info.dli_sname)" ... Fixes PVS Studio flagged issue: V547 Expression 'info.dli_sname == 0' is always false.
A very minor issue flagged by PVS Studio:
V815 Decreased performance. Consider replacing the
expression 'std::string("")' with 'std::string()'.
pos < out_buff_size is always true, because the
prior if-condition: if (out_buff_size == 0) { return; }
already guarantees that out_buff_size (an unsigned type)
is already non-zero, and long pos = 0, therefore
out_buff_size will always be greater than 0.
Fixes two PVS Studio issues:
V560 A part of conditional expression is always true:
pos < out_buff_size.
↑ V560 A part of conditional expression is always true:
pos < out_buff_size.
Fixes a PVS Studio issue: V522 There might be dereferencing of a potential null pointer 'with_newline'. Check lines: 1810, 1809.
Fixes two PVS Studio minor issues: V815 Decreased performance. Consider replacing the expression 's_arguments = ""' with 's_arguments.clear()'. V805 Decreased performance. It is inefficient to identify an empty string by using 'strlen(str) != 0' construct. A more efficient way is to check: str[0] != '\0'.
Fixes a PVS Studio issues: V823 Decreased performance. Object may be created in-place in the 's_user_stack_cleanups' container. Consider replacing methods: 'push_back' -> 'emplace_back'.
Fixes two issues flagged by Clang's static analyzer.
kcgen
changed the title
Fixes an error flagged by LGTM: Derived classes from a base class without a virtual destructor will only call the destructor of the type of the pointer being deleted and not the base-class. This can cause a defect if the pointer type is a base type while the object instance is a derived type.
Cleanup static analysis warnings
Fixes these warnings:
../../submodules/loguru/loguru.cpp:559:15: warning: comparison is always true due to limited range of data type [-Wtype-limits]
else if (0 <= c && c < 0x20) { // ASCI control character:
~~^~~~
../../submodules/loguru/loguru.cpp: In function ‘loguru::Text loguru::ec_to_text(char)’:
../../submodules/loguru/loguru.cpp:1776:14: warning: comparison is always true due to limited range of data type [-Wtype-limits]
else if (0 <= c && c < 0x20) {
~~^~~~
* Fix warning suppression on Windows clang When building loguru under Windows clang, _MSC_VER is defined, but it uses clang-style warning suppression flags. Fix that by checking for __GNUC__ and __clang__ first, then checking for _MSC_VER. Also fix the lack of a `#pragma GCC diagnostic pop` for people who include loguru.cpp. * Fix unsigned/signed comparison `snprintf` can return a negative value to signify that the buffer is not large enough to contain the bytes you attempted to write, so check for a positive value before advancing `pos` -- otherwise a later `snprintf` might overwrite bytes we've already written to the buffer.
Fixes two issues flagged by Clang's static analyzer.
|
Re-sync'd with latest changes in |
kcgen
force-pushed
the
staging/warning-cleanup-1
branch
from
8e6bb49
to
0155fe9
Compare
|
Recently merged PR#180 introduce two new PVS Studio warnings:
This branch now fixes these as well, in commit: dosbox-staging@0155fe9 |
kcgen
force-pushed
the
staging/warning-cleanup-1
branch
from
d73e358
to
57fcc3f
Compare
Fixes CI failure:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:35: note: ‘__builtin___snprintf_chk’ output between 5 and 11 bytes into a destination of size 5
64 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
65 | __bos (__s), __fmt, __va_arg_pack ());
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
CMakeFiles/loguru_test.dir/build.make:75: recipe for target 'CMakeFiles/loguru_test.dir/loguru_test.cpp.o' failed
make[2]: *** [CMakeFiles/loguru_test.dir/loguru_test.cpp.o] Error 1
CMakeFiles/Makefile2:82: recipe for target 'CMakeFiles/loguru_test.dir/all' failed
make[1]: *** [CMakeFiles/loguru_test.dir/all] Error 2
Makefile:100: recipe for target 'all' failed
make: *** [all] Error 2
CMake Error at test/appveyor.cmake:22 (_message):
kcgen
force-pushed
the
staging/warning-cleanup-1
branch
from
57fcc3f
to
29992d9
Compare
MSYS2's own headers define NOMINMAX in:
/msys64/mingw64/include/c++/11.2.0/x86_64-w64-mingw32/bits/os_defines.h
This causes a redefinition warning when building Loguru:
../src/libs/loguru/loguru.cpp:131: warning: "NOMINMAX" redefined
131 | #define NOMINMAX
|
In file included from D:/a/_temp/msys64/mingw64/include/c++/11.2.0/x86_64-w64-mingw32/bits/c++config.h:586,
from D:/a/_temp/msys64/mingw64/include/c++/11.2.0/utility:68,
from D:/a/_temp/msys64/mingw64/include/c++/11.2.0/algorithm:60,
from ../src/libs/loguru/loguru.cpp:33:
45 | #define NOMINMAX 1
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issues flagged by the Clang static analyzer, Coverity, and PVS Studio.
Suggest reviewing commit-by-commit, for which the following provides more background.
Uninitialized member variables, flagged by GCC and PVS Studio
Unused position increment (clarify intent), flagged by Clang's static analyzer
Simplify empty
std::stringoperations (1/2), flagged by PVS StudioSimplify empty
std::stringoperations (2/2), flagged by PVS StudioPotential nullptr dereference, flagged by PVS Studio
Clarify criteria by removing redundancy, flagged by PVS Studio
If it makes sense to keep keep them, I could see them be valid as assert
s orCHECK_F`s -- let me know.Virtualize EcEntryBase's destructor so it's called by derived classes, flagged by LGTM
Explicitly ignore sigaction's return value, flagged by Coverity