- I've also tried to provide fixes for some (until I got both tired and unable to fix those that result from the ASN.1 definitions) in
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5858.
- CLANG static analyzer was also quite helpful in finding issues. It just takes hours to compile wireshark with it.
- First time I've heard of ptvcursors. Nice.
Y.
On Thu, Jul 14, 2011 at 5:51 AM, Bill Meier
<wmeier@xxxxxxxxxxx> wrote:
On 7/13/2011 7:38 PM, Guy Harris wrote:
Haven't you (and maybe others) been fixing the same issues already,
as a result of Coverity warnings about the same thing?
And how many of those are
static void
dissect_whatever(...)
{
...
proto_tree_add_item(tree, hf_foo, tvb, offset, len_foo, encoding);
offset += len_foo;
proto_tree_add_item(tree, hf_bar, tvb, offset, len_bar, encoding);
offset += len_bar;
}
and how many of those ultimately represent dissectors that should be
converted to use ptvcursors, in which case the "offset +=" stuff will
disappear into the ptvcursor code and not get whined about by dataflow
analyzers?
{
...
offset += ...;
}
However, ISTR that many more were related to the following pattern:
{
...
foo = proto_tree_add_item(...);
offset += ...;
foo = proto_tree_add_item(...);
...
}
Others were real bugs (such doing 'foo=proto_item_add_subtree()' and then failing to use the returned value in following 'proto_tree_add_item()' calls.
At some point I stopped fixing "Coverity [unused]" even though ISTR that were (at least a few) more "unused" defects yet to be fixed.
Some time later I started working on the GCC 4.6 "set-but-unused" warnings.
I now see (after doing a little research) that it appears that the "unused" defects found by Coverity were only a subset of the "unused-but-set" cases found by GCC 4.6.
I don't know what the pattern is for things not found by Coverity but I do note that many cases like the following weren't found by Coverity:
{
...
int foo;
...
foo = ...;
...
}
In any case: Since much of the "Coverity [unused"" dissector cases have already been fixed, much of the remaining (non-generated) dissector "set-but-not-used" cases seem to be for stuff not found by "Coverity [unused]".
(See SVN #37716 for many examples of this type of fix).