Comment # 4
on bug 8472
from Florian Forster
Hi Michael and Evan,
thank you for reviewing the patch!
(In reply to comment #2)
> Also, please attach a sample capture to test with.
Done.
(In reply to comment #1)
> Do you really need the [abs|rel]_time_to_str calls? Can't you just use
> proto_tree_add_time and make those fields filterable instead of using
> proto_tree_add_text?
Each packet contains several sub-components. Each sub-component contains a type
(hostname, time, interval, …), a length and the actual payload.
The {abs,rel}_time_to_str() / proto_tree_add_text() calls are used in two
places. The first use is for the tree node representing the entire
sub-component. The type, length and payload are then added as children to this
node. These leaf nodes are added with proto_tree_add_time() at the end of the
dissect_collectd_integer() function, so the filtering does work.
proto_tree_add_text() is used so the "item" is added to the protocol tree with
the "header field" only once. Should I use proto_tree_add_time_format_value()
instead?
The second use is more cosmetic: Each packet can contain multiple metrics. Each
VALUES sub-component marks the end of the information for one metric. If some
information about a metric is identical for the previous metric, e.g. the same
hostname is used, this information is not re-transmitted. So in many packets
the hostname, interval, … is transmitted only once.
The VALUES sub-component includes a "simulation" in the tree which shows the
currently active values for all the appropriate fields, so that understanding
which values are actually transmitted is easier and doesn't involve scanning
the packet dissection backwards to find the last INTERVAL sub-component or
similar.
Here, too, _add_text() is used in order to skip referencing the "header field".
If you prefer, I can switch that to _add_time_format_value() too, if you say
that this is preferred.
HTH, thanks again and best regards,
—octo
You are receiving this mail because:
- You are watching all bug changes.