Bill Meier
changed
bug 8387
| What |
Removed |
Added |
| Status |
IN_PROGRESS
|
INCOMPLETE
|
Comment # 7
on bug 8387
from Bill Meier
Ok: 1 more round of fixes and I think I'll be OK committing:
Comments (in no particular order).
1. We prefer not to use "4 space tabs". (I do appreciate your taking the effort
to fix packet-zbee-zcl.c to use consistent indentation; I'll fix
packet-zbee-zcl.c to use consistent 4-space indentation).
Also: I believe that '-*- Mode: C; tab-width: 4 -*-' would need to be
the first line of the file to be effective (at least for EMacs).
I suggest using editor modelines like the following at the end of the file
instead.
/*
* Editor modelines - http://www.wireshark.org/tools/modelines.html
*
* Local variables:
* c-basic-offset: 4
* tab-width: 8
* indent-tabs-mode: nil
* End:
*
* vi: set shiftwidth=4 tabstop=8 expandtab:
* :indentSize=4:tabSize=8:noTabs=true:
*/
2. // comments aren't supported by all the compilers which Wireshark supports.
In some (all ?) cases in your patch, it appears that the line commented out
should just be removed.
3. The last arg of proto_tree_add_item() should specify the
encoding of the value: ENC_NA, ENC_LITTLE_ENDIAN, or ENC_BIG_ENDIAN.
(See doc/README.developer)
/* Add the command ID. */
//proto_tree_add_uint(tree, hf_zbee_zcl_on_off_srv_rx_cmd_id, tvb, offset,
sizeof(guint8), cmd_id);
proto_tree_add_item(tree, hf_zbee_zcl_on_off_srv_rx_cmd_id, tvb, offset,
sizeof(guint8), cmd_id);
In any case, it's no longer necessary to obtain the value (from whereever)
if the value is being fetched from the tvb via the use of
proto_tree_add_item() and is not being used elsewhere.
4. config.nmake was included by mistake in the patch.
5. #include <epan/prefs.h> isn't needed in packet-zbee-zcl-on-off.c
Ditto for string.h
6. It appears that packet-zbee-zcl-on-off.h is used only in
packet-zbee-zcl-on-ff.c.
If so, please put the defines in the .c file instead of using a separate
.h file.
You are receiving this mail because:
- You are watching all bug changes.