https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6428
--- Comment #4 from Anders Broman <anders.broman@xxxxxxxxxxxx> 2011-10-03 22:19:38 PDT ---
Hi,
A quick review.
- Please make it a built in dissector.
- No C++ style comments(//)(see README.developer on compabillity)
- Follow the style of other dissectors by placing proto_reg_handoff() an
proto_register() at the end of the file.
- +static const value_string packettypenames[] = {
pregix with brp
+static const value_string brp_packettype_names[] = {
- +static const value_string hf_brp_stat_vals[] = {
do not prefix with hf
+static const value_string brp_stat_vals[] = {
- #include <stdio.h> not needed.
- Declaring hf fields do not duplicate the field description
+ { &hf_brp_type,
+ { "Type", "brp.type", FT_UINT8, BASE_DEC, VALS(packettypenames), 0x0,
+ "Type", HFILL }},
Should be
+ { &hf_brp_type,
+ { "Type", "brp.type", FT_UINT8, BASE_DEC, VALS(packettypenames), 0x0,
+ NULL, HFILL }},
Run tools/checkhf.pl
- Do not use tvb_memcpy() use tvb_get_guint8()
- + proto_tree_add_item( brp_tree, hf_brp_type, tvb, offset, 1,
FALSE ); offset += 1;
I'd prefere that as two lines, FALSE should be ENC_BIG_ENDIAN as we are
chenging that.
- A reference to a protocol spec would be nice and an entry to the wiki
protocol pages.
- Using the standard boiler plate would be nice too
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@xxxxxxxxxxxxx>
* Copyright 1998 Gerald Combs
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
Best regards
Anders
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
