Coverity musings

[Jean-Marc Lasgouttes]

Le 14/09/2024 à 15:40, Pavel Sanda a écrit :
> Can you explain what that construct works?
> If I understand correctly we are changing the underlying structure
> and that does not feel right for "search" procedure.
> Apart from that if we search extensively the memory will grow as well?

First, for the first question: AFAIU, the loop is always over the 
columns of the tabular, so that 100 elements in the structure would 
already be extraordinary.

Back in history, the structure was not a map<col_type, bool> but a 
vecor<bool>. It is used to track things like "is there a line below this 
cell?". I am not sure why  Juergen changed it to a map, but the good 
thing about it is that now Coverity cannot complain that a value that it 
read/written might be outside of bounds ;)

A reasonable solution would have been a set<col_type> (containing the 
columns that are "on", whatever this means). However, this is not 
convenient since there are places where the value for a column is copied 
the the next one.

So, back to the map<>. A lot of code was like
	if (topline.find(c) != topline.end() && topline.find(c)->second)
that is "if we have data for this column and the data is "true" do 
something". Coverity was complaining in places where the first part of 
the test was missing. All this code was safe, though.

Now, the description of the [] operator says:
https://en.cppreference.com/w/cpp/container/map/operator_at
Inserts value_type(key, T()) if the key does not exist.
[...]
If an insertion is performed, the mapped value is value-initialized 
(default-constructed for class types, zero-initialized otherwise) and a 
reference to it is returned.

So, conveniently, the missing cells will be added and initialized to 
"false".

Having a vector<bool> should be equivalent and saner, except that we may 
have to convince Coverity that the bounds are respected.

What I wrote above is my understanding of the situation. My question 
about it is whether I miss something.

JMarc