Wireshark · Wireshark-dev: Re: [Wireshark-dev] proto_tree_add_item() with range_string

[Sebastien Tandel <sebastien@xxxxxxxxx>]

Hi,


> ...or have a BASE_RANGE_STRING flag that you OR with BASE_xxx.
>
> That means that the value in the structure isn't one of the enum  
> values, so the debugger won't be able to prettyprint it, though.
>   
Ack, as it was not the spirit of the enum structure (aka. DEC_HEX,
HEX_DEC) I didn't do it. Is the pretty print option of gdb, the reason
to create an enum (even for combinations)?
gdb newbie question : how do you pretty print the information once
you're inspecting the value of the field "display"?

>> VALS and TFS are independent from the base
>> chosen (or in general display field) and RVALS won't be.
>>
>> What do you think of an hybrid solution between mine and your defining
>> an additional field (i.e. convertfield_type) in head_register_info  
>> which
>> identifies which type of structure is in CONVERTFIELD (strings)?
>> - It would bring the independence  from the base avoiding us to
>> duplicate the fields (BASE_RANGE_STRING_DEC,  
>> BASE_RANGE_STRING_HEX, ...)
>> - this field could be hidden by the HFILL macro when you don't have to
>> use it (and use another, HFILL2(?), when you have to use this field)
>>     
>
> We might want to define macros to expand to definitions of fields of  
> particular types, with particular options; that'd also handle the  
> HFILL stuff, without having to have multiple HFILL macros.  Keep the  
> old HFILL and have it initialize that additional field to a default  
> value, and use the macros to create definitions that set it (and  
> convert existing definitions to use the new macros over time as  
> desired).
>   
Ack.

> Adding an additional field *does* make the structure a bit bigger,  
> which might worsen the memory and cache footprint of Wireshark;  
> whether it'd make a significant difference is another matter.
>
> Note also that other display options might be useful to attach to  
> fields, e.g. units, so you could define something such as
>
> 	HF_UINT_WITH_UNITS(hf_xxx_data_rate, "Data rate", "xxx.data_rate",  
> FT_UINT64, BASE_DEC, NULL, 0x0, "", "bytes/s")
>
> (as an example of the new type of macro I mentioned), which might  
> expand to something such as
>
> 	{ &hf_xxx_data_rate,
> 	{ "Data rate", "xxx.data_rate", FT_UINT64, BASE_DEC, NULL, 0x0, "Data  
> rate, in bytes/s", "bytes/s", HFILL }},
>
> so that the field would be displayed as "Data rate: %u bytes/s" rather  
> than just "Data rate: %u".  That might let us get rid of a number of  
> proto_tree_add_xxx_format() calls.  (Arguably, every time somebody  
> calls proto_tree_add_xxx_format(), it means we failed to anticipate a  
> need, although some might be sufficiently infrequently used that it's  
> not worth adding another display feature for it.)
>   
Ack for the feature but am I missing something? HFILL can't be used
anymore in your expanded example, right?

units (or suffix) is one example but there are some others (with high
frequence of presence) which are asking a specific format
- "%s %s" strings dynamically generated,
- "%4.4X"
- others I don't know ...
with only this field (units/suffix).


I tried to compile head_register_info with one more field (const gchar *
units and changed HFILL to init units to NULL). It compiles perfectly
but when I'm launching wireshark it segfaults :

#0  0xb63ac1db in strlen () from /lib/tls/libc.so.6
#1  0xb637eb00 in vfprintf () from /lib/tls/libc.so.6
#2  0xb637ac9a in cuserid () from /lib/tls/libc.so.6
#3  0xb637af5e in vfprintf () from /lib/tls/libc.so.6
#4  0xb6383d62 in fprintf () from /lib/tls/libc.so.6
#5  0xb6f9d148 in proto_register_field_array (parent=51904,
hf=0xb5ed1560, num_records=31) at proto.c:3506
#6  0xb5ecff2f in proto_register_agentx () at packet-agentx.c:1083
#7  0xb5ece4f7 in plugin_register () at plugin.c:20
#8  0xb6f9046b in plugins_scan_dir (dirname=0xb764161c
"/usr/local/lib/wireshark/plugins/0.99.5") at plugins.c:334
#9  0xb6f90657 in init_plugins () at plugins.c:406
#10 0xb6f9d2b4 in proto_init (register_all_protocols=0x805fa98
<register_all_protocols@plt>,
    register_all_protocol_handoffs=0x8060a08
<register_all_protocol_handoffs@plt>) at proto.c:325
#11 0xb6f89c4c in epan_init (register_all_protocols=0x3,
register_all_handoffs=0x3, report_failure=0x3, report_open_failure=0x3,
report_read_failure=0x3) at epan.c:97
#12 0x08081e05 in main (argc=1, argv=0xbf9ab6a4) at main.c:2364


What am I missing?

> I'm not sure whether there's a way to, for example, bundle all the  
> display information in a single structure, e.g. one with an optional  
> value_string or true_false_string or range_string table *and* a units  
> string and possibly even the base.
>
>   

While discussing about head_register_info structure, is there any chance
to see a mechanism of function callback in it?
I thought it would help to write dissectors (for example ip, tcp) in
which we can use the standard API (proto_tree_add_item, ptvcursor_add)
and be able to separate the structure of the packet from the treatments
made on some of its fields. We then would always be able as human to
read these dissectors easily and discerns the structure of the packets
even with heavy treatments made on packets fields (like in tcp).



Regards,

Sebastien Tandel