Wireshark-bugs: [Wireshark-bugs] [Bug 5749] Improve Connection Manager and Connection Configurat
Date: Tue, 10 May 2011 19:51:31 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5749

--- Comment #13 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2011-05-10 19:51:29 PDT ---
(In reply to comment #12)
> Created an attachment (id=6276)
 --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=6276) [details]
> CM and CCO dissector update, take 2
> 
> Did some refactoring and replaced a few of the proto_tree_add_text()s with
> proto_tree_add_items()s, so there's only 1 700+ line function remaining.

Good progress :-)

> To me proto_tree_add_text() vs proto_tree_add_items() comes down to whether or
> not the fields will be filtered in practice.  As a frequent/advanced user of
> the protocol,  I don't think the items that use proto_tree_add_text() will ever
> be filtered, so why add to the list?  It just makes the autocomplete that much
> more crowded.  I think I forced a few of the proto_tree_add_items().

I've always been surprised what users end up using.  Who would ever care about
the value in a field's padding, for example?  Well, I have--because the value
not being zero was the indication that "the bug" happened.  Because the value
of the padding bytes was a filter I could find the packets I needed with a few
keystrokes rather than painfully looking at 100,000 packets (okay, I would
never do that, I'd run the output of tshark through grep, but that would still
be a pain compared to just running a filter).

Filterable items can also be used in custom columns (as Stephen mentioned), I/O
graphs, and all sorts of other cool features--the stuff that make Wireshark
great.  Just decoding to text weakens all that.

If you're worried about having too many filters, check out X11:

grep -c '&hf_' epan/dissectors/x11-register-info.h 
7101

I'd still like to see most of the new add_text()'s turn into add_item()s (or
other filterable things).  (Fixing the old ones would be a bonus, of course.) 
That being said, I won't reject the patch in case another developer doesn't
feel as strongly as I do.

> Going back to the 700+ line functions, I feel that there is too much whitespace
> in those functions and that's the reason for their length.  The format seems to
> be:
> comment on field to be added
> field to add
> empty line
> 
> Since the hf_ variable or the "format text" of proto_tree_add_* give you a clue
> as to the field to add, some of the comments seem superfluous, but I didn't
> remove them as I take it as a "style difference" between developers.

Fair enough...

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