Evan Huus
changed
bug 8089
| What |
Removed |
Added |
| Status |
INCOMPLETE
|
IN_PROGRESS
|
Comment # 29
on bug 8089
from Evan Huus
(In reply to comment #28)
> Some minor issues:
>
> 1. I don't see much point to the #defines that just map to value_string
> values. Will they eventually be used in the code (via switch statements)?
> All but the function codes should just have the numeric value right in the
> value_string.
Stylistically I would tend to disagree - even if they're only used in the one
place I think the #defines are a good thing for readability/maintenance.
Regardless, the dissector isn't complete yet (there are a few other things that
aren't really used at the moment either) so they may end up used eventaully.
> 2. Patch should include not just the dissector file but the small
> modifications to /epan/MakeLists.txt and /epan/dissectors/Makefile.common to
> include it within the build.
Yes, technically. It's marginally easier to review the actual code without all
the patch boilerplate, but that is the style we normally ask for.
Neither issue is a blocker, so I will add the current version to the build
shortly.
You are receiving this mail because:
- You are watching all bug changes.