Wireshark-bugs: [Wireshark-bugs] [Bug 3534] IETF ForCES(Forwarding and Control Element Separatio

Date: Thu, 6 Sep 2012 05:50:16 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3534

Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darkjames-ws@xxxxxxxxxxxx

--- Comment #14 from Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx> 2012-09-06 05:50:15 PDT ---
(In reply to comment #13)
> (In reply to comment #11)
> > col_set_str(pinfo->cinfo, expert_add_info_format, ... must be also called when
> > tree == NULL.
> 
> I don't understand this comment.  I thought most tree == NULL checks where for
> still processing a packet even though you didn't need it displayed (like for
> converstations, states, etc).  This dissector is strictly for "display", which
> is why the tree == NULL check is so early. 

Only if you want to display tree (heh, it's quite obvious).
If you want to display only columns info, *sometimes* tree is NULL, check:

1/ packet_list_dissect_and_cache_record() (ui/gtk/packet_list_store.c)

1186 create_proto_tree = (dissect_color && color_filters_used()) ||
1187                     (dissect_columns && have_custom_cols(cinfo));
1188 
1189 epan_dissect_init(&edt,
1190                        create_proto_tree,
1191                        FALSE /* proto_tree_visible */);

2/ process_packet (tshark.c)

3066 if (cf->rfcode || verbose || filtering_tap_listeners ||
3067    (tap_flags & TL_REQUIRES_PROTO_TREE) || have_custom_cols(&cf->cinfo))
3068      create_proto_tree = TRUE;
3069 else
3070      create_proto_tree = FALSE;
/* ... */
3076  epan_dissect_init(&edt, create_proto_tree, print_packet_info && verbose);

In gtk create_proto_tree depends on colors, so tree == NULL is unlikely.
But in tshark it should be common, just don't pass -V.

Similar case for expert info, 'tshark -q -z expert' don't require tree
creation.

>From ui/cli/tap-expert.c:
255     error_string = register_tap_listener("expert", hs,
256                                          filter, ***0***,

Compare with ui/gtk/expert_comp_dlg.c:
885     error_string=register_tap_listener("expert", etd, NULL /* fstring */,
886                                        ***TL_REQUIRES_PROTO_TREE***,


So checking for tree == NULL, and not setting column info/expert is mostly ok
in wireshark, in other application using libwireshark it might be not.

AFAIK we care about other applications, quoting README.developer:
"Note, however, that you must fill in column information, (...)
 do calls to "expert" functions, (...) regardless of whether 'tree'
 is NULL or not."

> Should I just move the 
> col_set_str(pinfo->cinfo, COL_PROTOCOL,  "ForCES");
> col_clear(pinfo->cinfo, COL_INFO);
> 
> lines into dissect_forces_tcp() and dissect_forces_not_tcp()?

I'd rather remove tree == NULL check, but it's your patch.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.