Hi,
So I spent some quality time in front of Coverity and here is the
result: since the start of this week, the number of outstanding defects
is down from 246 to 87.
This is in addition to the 75 identical defects that got killed in one
fell swoop last week.
How was that done?
* fixes, aka the commits you saw on the list. Several strategies here:
- pass parameters as const references when possible. I did that even
for shared_ptr<> objects, after reading a bit about this.
- rewrite code to make sure that Coverity sees that nothing was wrong
after all
- Fix some actual bugs
* all the performance issues concerning string, docstring and QString,
where we do a copy, and it would be possible to use std::move() instead,
have been closed as "Ignored".
* some other copies of Qt objects which are known to use COW have been
ignored too.
So what remains now?
* still lots of places where Coverity wants us to use std::move because
the variable that is copied will not be used any more. I tend to think
that we should avoid that and reduce our use of move() to the minimum,
unless profiling shows that we have an issue. Do we have an agreement on
that?
* Places where I understand the issue, but fixing requires some work and
is a bit scary. Example in the patch below: Jürgen, could you please
have a look and tell me why we should not do that? It is too simple, I
fear I am missing something.
* Places where I understand the program, but am not sure of a proper fix.
* Places where I do not even understand what the issue is :(
Have a nice weekend.
JMarc
From 213d8088bcbcae78ad735bb46bc8b1fb93e75bca Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Fri, 13 Sep 2024 17:10:58 +0200
Subject: [PATCH] Streamline use of map<col-type, bool>
Typically, to check whether an element is present and set to true, instead of
foo.find(c) != foo.end() && foo.find(c)->second
one uses
foo[c]
The map<> code creates elements automatically when they do not exist
and zero-initializes them.
This avoids puzzlement of Coverity scan when seeing that find() result
is dereferenced without checking that it is not equal to end().
Moreover, this makes the code much much clearer. I still do not
understand it, though ;)
---
src/insets/InsetTabular.cpp | 53 ++++++++++++++-----------------------
1 file changed, 20 insertions(+), 33 deletions(-)
diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index a36808fd56..e9050c4fb3 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -2634,10 +2634,9 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, list<col_type> const &
topltrims[c] = topltrims[c-1];
toprtrims[c] = toprtrims[c-1];
}
- if (topline.find(c) != topline.end() && topline.find(c)->second)
+ if (topline[c])
++nset;
- if ((topltrims.find(c) != topltrims.end() && topltrims.find(c)->second)
- || (toprtrims.find(c) != toprtrims.end() && toprtrims.find(c)->second))
+ if (topltrims[c] || toprtrims[c])
have_trims = true;
}
@@ -2669,7 +2668,7 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, list<col_type> const &
if (cl < c)
continue;
c = cl;
- if (topline.find(c)->second) {
+ if (topline[c]) {
col_type offset = 0;
for (col_type j = 0 ; j < c; ++j)
if (column_info[j].alignment == LYX_ALIGN_DECIMAL)
@@ -2679,21 +2678,18 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, list<col_type> const &
while (isPartOfMultiColumn(row, c))
++c;
string trim;
- if (topltrims.find(c) != topltrims.end()
- && topltrims.find(c)->second)
+ if (topltrims[c])
trim = "l";
col_type cstart = c;
- for ( ; c < ncols() - 1 && topline.find(c + 1)->second ; ++c) {
+ for ( ; c < ncols() - 1 && topline[c + 1] ; ++c) {
if (isMultiColumn(cellIndex(row, c))
&& c < ncols() - 1 && isPartOfMultiColumn(row, c + 1))
continue;
- if (c > cstart && topltrims.find(c) != topltrims.end()
- && topltrims.find(c)->second) {
+ if (c > cstart && topltrims[c]) {
if (!isPartOfMultiColumn(row, c))
--c;
break;
- } else if (toprtrims.find(c) != toprtrims.end()
- && toprtrims.find(c)->second)
+ } else if (toprtrims[c])
break;
}
@@ -2701,8 +2697,7 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, list<col_type> const &
if (column_info[j].alignment == LYX_ALIGN_DECIMAL)
++offset;
col_type lastcol = (*it1 == *it2) ? c + 1 + offset : columns.size() - c + offset;
- if (toprtrims.find(c) != toprtrims.end()
- && toprtrims.find(c)->second)
+ if (toprtrims[c])
trim += "r";
os << cline;
@@ -2761,8 +2756,8 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, list<col_type> const
bottomltrims[c] = bottomltrims[c-1];
bottomrtrims[c] = bottomrtrims[c-1];
}
-
- nextrowset &= topline.find(c) != topline.end() && topline.find(c)->second;
+
+ nextrowset &= topline[c];
}
// combine this row's bottom lines and next row's toplines if necessary
@@ -2770,15 +2765,12 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, list<col_type> const
bool have_trims = false;
for (auto const & c : columns) {
if (!nextrowset)
- bottomline[c] = bottomline.find(c)->second || topline.find(c)->second;
- bottomltrims[c] = (bottomltrims.find(c) != bottomltrims.end() && bottomltrims.find(c)->second)
- || (topltrims.find(c) != topltrims.end() && topltrims.find(c)->second);
- bottomrtrims[c] = (bottomrtrims.find(c) != bottomrtrims.end() && bottomrtrims.find(c)->second)
- || (toprtrims.find(c) != toprtrims.end() && toprtrims.find(c)->second);
+ bottomline[c] |= topline[c];
+ bottomltrims[c] |= topltrims[c];
+ bottomrtrims[c] |= toprtrims[c];
if (bottomline.find(c)->second)
++nset;
- if ((bottomltrims.find(c) != bottomltrims.end() && bottomltrims.find(c)->second)
- || (bottomrtrims.find(c) != bottomrtrims.end() && bottomrtrims.find(c)->second))
+ if (bottomltrims[c] || bottomrtrims[c])
have_trims = true;
}
@@ -2803,7 +2795,7 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, list<col_type> const
if (cl < c)
continue;
c = cl;
- if (bottomline.find(c)->second) {
+ if (bottomline[c]) {
col_type offset = 0;
for (col_type j = 0 ; j < c; ++j)
if (column_info[j].alignment == LYX_ALIGN_DECIMAL)
@@ -2813,23 +2805,19 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, list<col_type> const
while (isPartOfMultiColumn(row, c))
++c;
string trim;
- if (bottomltrims.find(c) != bottomltrims.end()
- && bottomltrims.find(c)->second)
+ if (bottomltrims[c])
trim = "l";
col_type cstart = c;
- for ( ; c < ncols() - 1 && bottomline.find(c + 1)->second ; ++c) {
+ for ( ; c < ncols() - 1 && bottomline[c + 1] ; ++c) {
if (isMultiColumn(cellIndex(row, c))
&& c < ncols() - 1
&& isPartOfMultiColumn(row, c + 1))
continue;
- if (c > cstart
- && bottomltrims.find(c) != bottomltrims.end()
- && bottomltrims.find(c)->second) {
+ if (c > cstart && bottomltrims[c]) {
if (!isPartOfMultiColumn(row, c))
--c;
break;
- } else if (bottomrtrims.find(c) != bottomrtrims.end()
- && bottomrtrims.find(c)->second)
+ } else if (bottomrtrims[c])
break;
}
@@ -2837,8 +2825,7 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, list<col_type> const
if (column_info[j].alignment == LYX_ALIGN_DECIMAL)
++offset;
col_type lastcol = (*it1 == *it2) ? c + 1 + offset : columns.size() - c + offset;
- if (bottomrtrims.find(c) != bottomrtrims.end()
- && bottomrtrims.find(c)->second)
+ if (bottomrtrims[c])
trim += "r";
os << cline;
--
2.34.1
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
