Wireshark-bugs: [Wireshark-bugs] [Bug 5013] Wireshark doesnt support FEC 129(0x81) TLV in LDP me
Date: Sun, 10 Oct 2010 18:06:40 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5013

Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jeff.morriss.ws@xxxxxxxxx

--- Comment #11 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2010-10-10 18:06:28 PDT ---
Sorry--I don't think many of us have a lot of free time these days.

Unfortunately, the patch no longer applies cleanly: could you update to the
latest SVN again?

BTW, the checks like "if(fec_tree == NULL) return" could be simplified or
mostly removed: the tree will either be NULL (at the beginning of the function)
or not.  If it's not NULL, proto_tree_add_subtree() won't generate a NULL tree
either.  Either way, checking for the tree being NULL is strictly optional at
this point.

You should probably also remove checks like:

+                if ( (vc_len > 1) && ( rem > 1 ) ) { /* there is enough room
for TAII */
[...]
+                } else {
+                    proto_tree_add_text(fec_tree,tvb,offset , 2 +vc_len,
"Generalized FEC: TAII size format error");
+                    return;
+                }

That's not really necessary: Wireshark will throw an exception if you try to
access packet data that's not there and the packet will helpfully show up as
Malformed.  If you really want it there, adding the information as an expert
info (with expert_add_info_format()) would be better than just adding it to the
tree (that way the packet will be flagged as being in error).

One last minor nitpick: dissect_tlv_pw_status() and the other function
can/should be moved higher: by convention the proto_reg_handoff function should
be last (or nearly so) in the file.  We could obviously just move it while
checking it in, but since you have to regenerate the patch anyway...

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