Wireshark-bugs: [Wireshark-bugs] [Bug 7658] LLRP: Dissect parameter fields
Date: Thu, 23 Aug 2012 18:11:28 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7658

--- Comment #8 from Evan Huus <eapache@xxxxxxxxx> 2012-08-23 18:11:27 PDT ---
After taking a bit more time to understand what they're doing, I don't have any
real problem with the macros. Ideally I think they wouldn't be there - I feel
they add more in complexity than they save in typing - but that's really a
judgement call.

If you're willing to do the work then I'm more than happy to take a patch to
replace them - the state of the dissector before your first patch should be a
good guide, or you can take a look at one of the small dissectors like moldudp.

I forgot to answer this before, but you asked about the duplication between
declaring the "static int hf_llrp_* = -1;" and actually registering the fields
in proto_register_llrp. I tend to think of the first one as a
forward-declaration, just like you do with functions, and then the second one
is the definition. Ideally you'd only need one, but ideally you wouldn't need
to forward-declare functions either - it's just the way the language works.

If you decide not to remove the macros, I would appreciate a small patch adding
some comments around them so that the next person to read through the dissector
doesn't have to figure them out on his/her own.

Thanks,
Evan

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