Eliminate defects found by Coverity #16
Conversation
Coverity suggests to pass the argument by move instead of copy. Refactor it to follow the convention of using const ref for such arguments instead.
Sugggested by Coverity (CID 449636, CID 449638).
| , m_asyncRunning(false) | ||
| , m_notified(false) | ||
| , m_complete(false) | ||
| , m_rowsChanges(false) |
There was a problem hiding this comment.
Not doing this is definitely an improvement.
| @@ -117,26 +117,26 @@ public slots: | |||
|
|
|||
| QString m_asyncErrorString; | |||
| QVector<TransferDBRecord> m_asyncRows; | |||
| QVector<TransferDBRecord> *m_rows; | |||
| QVector<TransferDBRecord> *m_rows{}; | |||
There was a problem hiding this comment.
Is this really the preferred syntax? I find this quite unintuitive.
There was a problem hiding this comment.
Yes, search for "C++ uniform initialization".
There was a problem hiding this comment.
Not a fan of the syntax myself either, though initialization by the declaration is nicer than initialization lists.
One thing I'm pondering, though, is whether we should be using more explicit values on these, e.g. '= nullptr'; which should have the same result. We've been doing that to some extend already. Some characters more but could be nicer to read. Granted that I might get used to these eventually too.
There was a problem hiding this comment.
Everything is better than initializer lists :)
I tend to prefer:
type myMember = type();
or
type myMember = someDefaultValue;
There was a problem hiding this comment.
I have been using '= foo' too, because the new syntax was not easy to adapt to. But in general, '= foo' does not do the same as '{foo}', and the latter is less likely to cause issues, so it should be better choice when consistency is desired. It is also suggested by the C++ Core Guidelines https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax.
There was a problem hiding this comment.
Sure there are differences in going in depth of these, but what's the effective difference when talking about default values for POD member variables? More complex types should be anyway handled in the .cpp side.
The linked guidelines don't seem to explicitly cover the class declaration, but rather the generic instantiation of variables.
Pardon a bit of bike-shedding here, but maybe better do it once and not in all the forthcoming places getting coverity treatment :)
There was a problem hiding this comment.
@martyone: Right, thus my first preference of type().
Also: can't aggregate initialization (if this is indeed a sub-case of that) accidentally bypass use of proper/public constructors?
https://stackoverflow.com/questions/65580725/aggregate-initialization-by-passes-private-class-constructor
There was a problem hiding this comment.
@pvuorela, actually it is so that the linked guidelines simply make no exception with class declarations, they cover all cases with single rule. An example related to member initialization exists a bit more down, following that same rule https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-default-member-initializers-to-member-initializers-in-constructors-for-constant-initializers.
@attah, this is quite a special use case and addressed in C++20 as mentioned in the answer.
OTOH, QtC's coding style, which is closer to us, prefers assignment for simple types, so updated.
There was a problem hiding this comment.
Alright, I'm good with this.
Few cases of uninitialized members discovered by Coverity (CID 449633, CID 449640) and there was already one related fix few commits back, so better uniformly use default member initializers to ensure all members get reasonable defaults. The less exceptions the better, so even d_ptr etc. Also drop some code the compiler can generate automatically.
No description provided.