Wireshark · Ethereal-dev: [ethereal-dev] Getting rid of "proto_tree_add_item()"?

[Guy Harris <gharris@xxxxxxxxxxxx>]

Whilst doing some regression tests on the pile of capture files I have
in order to check the tvbuff code, I came across a bug in the Cisco VTP
dissector - I was passing "&pd[offset]", i.e. a pointer to the 32-bit IP
address in the "Updater Identity" field, rather than passing the 32-bit
IP address itself, to "proto_tree_add_item()".

Unfortunately, as "proto_tree_add_item()" is a varargs function, and the
actual value of the item to be added is one of the variable arguments,
the compiler couldn't catch this and print a warning.

I seem to remember that when we replaced "proto_tree_add_item_format()"
with a bunch of functions for different item types, so that those
functions would have *only* the format string and its arguments as
variable arguments and we could tell GCC to do format string/argument
checking, we found a bunch of bugs where the item value itself was being
passed incorrectly, as the change meant that the item value wasn't a
variable argument and the compiler could check its type.

I suspect we should do the same for "proto_tree_add_item()" and
"proto_tree_add_item_hidden()" - especially given that we're already
changing all the dissectors anyway to handle tvbuffs - and I suspect
we'll catch a number of bugs by doing so.

(We could also add a "string length" argument to the routines to add
string items, as per my suggestions earlier for dealing with counted
strings.)