https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7658
Evan Huus <eapache@xxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |eapache@xxxxxxxxx
--- Comment #2 from Evan Huus <eapache@xxxxxxxxx> 2012-08-21 18:21:49 PDT ---
Hi Martin, thanks for the patch. I'm afraid I can't comment on the modification
to proto.c - hopefully somebody else will be able to take a look at that - but
I've given the rest of the patch a quick review. No major problems I don't
think, but a few nits:
- Is there any particular reason the LLRP_COMM_STANDARD #defines aren't
capitalized?
- You use a couple of variadic macros. Unfortunately, not all compilers support
those (this is documented in an obscure corner of /doc/README.developer).
- Speaking of macros, you do some interesting things with them that I'm not
necessarily comfortable with. I'll admit that I haven't taken the time to fully
understand them yet, but it would make me a lot happier if you replaced them
with more traditional dissector code. I'm not sure how much typing some of them
saved you, anyways...
- There are a couple of places (the switch case for
LLRP_IMPINJ_PARAM_REDUCED_POWER_FREQUENCY_LIST caught my eye) where you get a
value from the TVB and loop over it, adding a new tree item each time. Before
looping you should check the value you got against the length of the buffer, as
otherwise it's possible to effectively hang Wireshark by putting an enormous
value in that field.
Cheers,
Evan
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
