Wireshark-bugs: [Wireshark-bugs] [Bug 4966] Added support for Organization Specific Slow Protoco
Date: Mon, 5 Jul 2010 06:30:11 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4966

--- Comment #2 from Roberto Morro <roberto.morro@xxxxxxxxxxxxxxxx> 2010-07-05 06:29:54 PDT ---
(In reply to comment #1)
> quick review:
> check_col() is deprecated.
[RM] I'm a little bit confused here, because README.developer states "Before
changing the contents of a column you should make sure the column is active by
calling 'check_col'....". There's absolutely no problem in removing the call
(in my view the code becomes even more readable), but should I remove it only
in my patch or throughout the entire packet-slowprotocols.c file?

> don't hide setting column info in 'if (tree) {' block.
[RM] OK.

> Can you avoid using tvb_get_ptr()
[RM] It looked to me the most efficient way to check for OUI, as done in other
part of this dissector and in other dissectors. Alternatively, I have to copy
the three OUI bytes into a local variable and then pass the pointer to the
local variable to the get_manuf_name() function. I have a slight preference for
the first option, but I'm open to the second.

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