Hi Pecan,
thank you for the effort.
With the last patch (svn13671 BrowseTracker Move ... ) I get a compile error when building on Linux with make. An include is missing:
When scrolling very briefly through the diff I noticed a function with a suspiciously empty if statement. It looks like unintentional semantics:
void JumpTracker::OnLeftUp(wxMouseEvent& event) {
if (m_leftDown && (not m_isDragging)) {
// Pure click (no drag) occurred
}
m_leftDown = m_isDragging = false;
event.Skip();
}
When I run CppCheck over the BrowseTracker cb-project, I get several warnings. E.g.:
BrowseTrackerLayout.cpp:173:41: warning: Either the condition 'cursor' is redundant or there is possible null pointer dereference: cursor. [nullPointerRedundantCheck]
TiXmlElement* browsemarks = cursor->NextSiblingElement("BrowseMarks");
^
BrowseTrackerLayout.cpp:155:17: note: Assuming that condition 'cursor' is not redundant
if (cursor)
^
BrowseTrackerLayout.cpp:173:41: note: Null pointer dereference
TiXmlElement* browsemarks = cursor->NextSiblingElement("BrowseMarks");
^
or
BrowseSelector.cpp:93:17: warning: Member variable 'BrowseSelector::m_pBrowseTracker' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior. [uninitMemberVarPrivate]
BrowseSelector::BrowseSelector()
^
BrowseSelector.cpp:93:17: warning: Member variable 'BrowseSelector::m_menuID' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior. [uninitMemberVarPrivate]
BrowseSelector::BrowseSelector()
^
BrowseSelector.cpp:93:17: warning: Member variable 'BrowseSelector::m_KeyDownCode' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior. [uninitMemberVarPrivate]
BrowseSelector::BrowseSelector()
^
BrowseSelector.cpp:93:17: warning: Member variable 'BrowseSelector::m_KeyDownMods' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior. [uninitMemberVarPrivate]
BrowseSelector::BrowseSelector()
^
Besides other warnings.
Since the topic-starter observed a crash, these should be fixed.
Have a nice day.
Thank you Tim,
I always build without precompiled headers.
The actual error from gcc is:
trunk/src/plugins/contrib/BrowseTracker/JumpTracker.cpp:1313:9: error: 'LogSlot' was not declared in this scope
1313 | LogSlot& logslot = pLogMgr->Slot(logIndex);
| ^~~~~~~
I found its definition in logmanager.h.
Should my include statement use quotes instead of <> brackets?
Yes, you are right. Its a project header, not a system header. Is is not done consistently in cb and the plugins though.
Those variables were actually initialized, but the code got smushed without white space so that cppcheck didn't recognize it.
I re-formated it to cppcheck taste.
Maybe in this case. I did not look it up before posting. Nevertheless there are numerous bad examples still in cb-code. As for BrowseSelector: The two ctors leave parts uninitialized and do not delegate. The default ctor is marked private, implemented but the comment says it is unused. I prefer code that does not pose additional puzzles. If a tool has already spotted an issue and it is not a false positive. Then why should I fire up the debugger and search for spurious errors due to UB.
I actually like having CppCheck (trunk) in addition to gcc, clang, msvc, scan-build, clangd, clang-tidy and the sanitizers.
It is a very well maintained project with constant little improvements. You can get more control over it with:
--inline-suppr --suppress=missingIncludeSystem
I have a largish project, north of 500k SLOC. In that I am usually down to a dozen cppcheck warnings. Granted, the code contains some cppcheck-suppress. Please note: It also contains some NOLINTNEXTLINE. I have enabled many warnings in compiler, cppcheck and clang-tidy. Some warnings might be debatable or not apply to some cases in a codebase. I prefer to not kill warnings globally but document exceptions on a per case base right at the spot in code. Needs judgement but that is a good thing. Recently, cppcheck changed the semantics of paths for #include. At the moment I get numerous missingInclude warnings. Did not yet investigate on that. Slightly annoying, hopefully just temporarily.
Thanks for adding the missing guard against nullptr. I would prefer code where this is not needed in the first place. With old code, you can not avoid it completely. But in newer code I'd at least try to minimize is.
If you are under Linux, I recommend to try scan-build( see https://clang.llvm.org/docs/analyzer/user-docs/CommandLineUsage.html#scan-build (https://clang.llvm.org/docs/analyzer/user-docs/CommandLineUsage.html#scan-build)).
It reports several bugs in cb.
Compilation on MSW now fails due to missing logmanager.h. The MSW project defines NOPCH, so sdk.h will not include logmanager.h. Also, double quotes should be used for the includes.
The solution is changing the first lines of JumpTracker.cpp to
#include "sdk.h" // Code::Blocks SDK
#include "configurationpanel.h"
#include "cbstyledtextctrl.h"
#include "logmanager.h"
#include "projectmanager.h"
#include "editormanager.h"
#include "cbeditor.h"