https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3534
Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jeff.morriss.ws@xxxxxxxxx
--- Comment #6 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2010-10-08 19:40:04 PDT ---
FWIW, this is now RFC 5810.
Yes, the num2* functions can and should easily be replaced with value_strings.
Other comments:
- Ethereal has been Wireshark for several years now (applies to the copyright
notice)
- all the tvb_get_xxx() calls followed by immediately by
proto_tree_add_xxx_format() should be replaced with proto_tree_add_item().
Adding value strings to the hf fields will make the displays come out right.
- all (or certainly most of) the length checks should be removed: there's no
need. Wireshark will work better if you just keep trying to access malformed
packet--eventually an exception will be thrown and the packet will show up as
malformed. This will save a lot of code and complexity (like the 'back'
variables + returns). A reasonable goal would be the elimination of
error_report_length(), though there may be some places where it would still be
useful.
- the check_col() calls can all come out (they're no longer necessary)
- all of the functions should be static (so they don't pollute the global name
space)
- next_tvb can be a function-local variable (instead of a non-static global)
- I think (but I'm not sure) that all of the function-static variables don't
need to be static
- Don't use temp, temp1, and temp2 over and over again through a function.
Define variables for each thing so the variable name tells you what it is (this
alleviates the need for a comment to say what's in the variable). If a
function starts to have too many variables, that's a good sign it's time to
make another function.
- Instead of doing this:
if(!abs(strcmp(num2tlvtype(type), "PATH DATA-TLV")))
or:
if(abs(strcmp(num2operationtype(type), "Unknown Operation Type"))
just use some macros (like TLV_PATH_DATA) and do an integer comparison. Or,
for the latter, use the return of match_strval() to determine if it's a known
operation.
- The days of 80-column code are probably gone, but 300+ character lines are
too long--there need to be some line wraps in here. (Eliminating those length
checks and/or making sub-functions would help.)
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
