Wireshark-bugs: [Wireshark-bugs] [Bug 6428] BRP Dissector
Date: Mon, 3 Oct 2011 22:19:40 -0700 (PDT)
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.
- References:
- [Wireshark-bugs] [Bug 6428] New: BRP Dissector
- From: bugzilla-daemon
- [Wireshark-bugs] [Bug 6428] New: BRP Dissector
- Prev by Date: [Wireshark-bugs] [Bug 6428] BRP Dissector
- Next by Date: [Wireshark-bugs] [Bug 6428] BRP Dissector
- Previous by thread: [Wireshark-bugs] [Bug 6428] BRP Dissector
- Next by thread: [Wireshark-bugs] [Bug 6428] BRP Dissector
- Index(es):
- Get Wireshark
- Download
- Code of Conduct