More coverity fixes #579
More coverity fixes #579
Conversation
|
Can one of the admins verify this patch? |
|
@phsft-bot build! |
|
Starting build on |
| fOperOptimized = 0; | ||
| fOperOffset = 0; | ||
| fPredefined = 0; | ||
| fOptimal = (ROOT::v5::TFormulaPrimitive::TFuncG)&TFormula::EvalParOld; |
There was a problem hiding this comment.
Why do we have these changes? Are they only whitespace changes?
hist/hist/src/TFormula_v5.cxx
Outdated
| // coverity[uninit] the tab value of tab is guaranteed to be set properly by the control flow. | ||
| Double_t tab[kMAXFOUND]; | ||
| const char *stringStack[gMAXSTRINGFOUND]; | ||
| Double_t tab[kMAXFOUND] = {0}; |
There was a problem hiding this comment.
Why do we have comment and initialization? I'd assume that one of them could go away?
There was a problem hiding this comment.
These initializations cost time. @lmoneta - are they actually needed or is Coverity incorrect here? The comment seems to claim Coverity is wrong.
There was a problem hiding this comment.
I think @pcanal added these comments, so he might know.
I think Axel is wight and we should ignore Coverity here
There was a problem hiding this comment.
The last time I checked, the control flow insure it was initialized before use. Given that this a deprecated class, I would not change.
Albeit, it looks like the comment needs to be updated for the current version of coverity and/or the report be flagged as false positive.
|
@phsft-bot build! |
|
@Axel-Naumann, isn't the reference to |
|
@vgvassilev what do you mean? This PR is proposing changes to formula_v5 and @axel ask clarification on them .... [note: this source file is used to read 'old' ROOT files] |
|
What should we do with this PR? Should I cherry-pick the approved changes and merge? |
|
I'd say merge as is. The whitespace changes in TFormula_v5 are just a tiny annoyance compared to leaving the members uninitialized. |
|
Should we then go ahead with the merge of this PR? |
|
Yes +6-12-patches, thanks! |
9 participants
No description provided.