Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 8275] Basic dissector: FIPA/ACL Message protocol over TCP

[bugzilla-daemon@xxxxxxxxxxxxx]

Evan Huus changed bug 8275

What	Removed	Added
Status	IN_PROGRESS	INCOMPLETE

Comment # 16 on bug 8275 from Evan Huus

Hi Raúl, thanks for your work on the update. I've had time to take a look at
the new version of the dissector and it's pretty good already, but there a few
issues remaining:

- The indentation is still not entirely consistent. Please add modelines to the
bottom of the file (https://www.wireshark.org/tools/modelines.html).

- The preferences implementation is a bit non-standard:
  - You register some obsolete preferences, which is odd since the dissector
isn't included in any previous versions of Wireshark. Were you distributing it
as a plugin before? (This is fine, but a comment explaining would be helpful).
  - Usually the proto_reg_handoff_acl function is used as the prefs callback
(instead of a separate reinit_acl). This is also where dissector handles (like
acl_handle and xml_handle) should be created or discovered. Take a look at the
file doc/packet-PROTOABBREV.c for an example of the usual style.
  - There's no need for the enable_acl_dissection preference. Wireshark keeps a
master list of enabled/disabled protocols already so that individual dissectors
don't have to implement that.

- We already provide an array_length macro defined in packet.h, there's no need
to have your own acl_array_length.

- Some of your tables (like acl_performatives and others) can probably be made
const.

- Please include in your patch the addition of the dissector to the appropriate
Makefiles (see section 1.9 of doc/README.developer).

- In some places (such as acl_get_bounds, though there may be others) it
probably makes sense to use tvb_reported_length() instead of just tvb_length(),
as this will behave better with captures that were truncated.

- Finally, it doesn't compile on my Linux system (my compiler is somewhat more
strict than the default Windows one). Here's a copy of some of the errors:

packet-acl.c:183:34: error: declaration of 'log' shadows a built-in function
[-Werror=shadow]
packet-acl.c:236:2: error: initialization discards 'const' qualifier from
pointer target type [-Werror]
packet-acl.c:264:6: error: static declaration of 'displayACLMessage' follows
non-static declaration
packet-acl.c:117:6: note: previous declaration of 'displayACLMessage' was here
packet-acl.c: In function 'displayACLMessage':
packet-acl.c:288:4: error: request for implicit conversion from 'void *' to
'struct acl_marker_t *' not permitted in C++ [-Werror=c++-compat]
packet-acl.c:298:5: error: request for implicit conversion from 'void *' to
'struct acl_marker_t *' not permitted in C++ [-Werror=c++-compat]
packet-acl.c:296:11: error: variable 'field_name' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:269:7: error: variable 'l' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:268:7: error: unused variable 'i' [-Werror=unused-variable]
packet-acl.c: At top level:
packet-acl.c:323:6: error: static declaration of 'acl_set_pinfo' follows
non-static declaration
packet-acl.c:119:6: note: previous declaration of 'acl_set_pinfo' was here
packet-acl.c:340:6: error: static declaration of 'acl_process_performative'
follows non-static declaration
packet-acl.c:120:6: note: previous declaration of 'acl_process_performative'
was here
packet-acl.c: In function 'acl_process_performative':
packet-acl.c:358:4: error: format not a string literal and no format arguments
[-Werror=format-security]
packet-acl.c: At top level:
packet-acl.c:371:10: error: static declaration of 'acl_get_acl_hf_ett' follows
non-static declaration
packet-acl.c:110:18: note: previous declaration of 'acl_get_acl_hf_ett' was
here
packet-acl.c:386:6: error: static declaration of 'acl_get_bounds' follows
non-static declaration
packet-acl.c:130:6: note: previous declaration of 'acl_get_bounds' was here
packet-acl.c: In function 'acl_get_bounds':
packet-acl.c:405:2: error: comparison of unsigned _expression_ >= 0 is always
true [-Werror=type-limits]
packet-acl.c: At top level:
packet-acl.c:421:8: error: static declaration of 'acl_str_rem_spaces' follows
non-static declaration
packet-acl.c:131:7: note: previous declaration of 'acl_str_rem_spaces' was here
packet-acl.c:433:6: error: static declaration of 'parseACLContent' follows
non-static declaration
packet-acl.c:113:6: note: previous declaration of 'parseACLContent' was here
packet-acl.c: In function 'parseACLContent':
packet-acl.c:449:8: error: variable 'savedState' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:446:9: error: variable 'cp' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:437:7: error: variable 'l' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c: At top level:
packet-acl.c:564:5: error: static declaration of 'dissect_acl_message_v2'
follows non-static declaration
packet-acl.c:115:5: note: previous declaration of 'dissect_acl_message_v2' was
here
packet-acl.c: In function 'acl_dissector_add':
packet-acl.c:685:52: error: unused parameter 'handle'
[-Werror=unused-parameter]
packet-acl.c: At top level:
packet-acl.c:695:6: error: static declaration of 'acl_dissector_log' follows
non-static declaration
packet-acl.c:132:6: note: previous declaration of 'acl_dissector_log' was here
packet-acl.c:707:10: error: static declaration of 'acl_message_likelyhood'
follows non-static declaration
packet-acl.c:111:18: note: previous declaration of 'acl_message_likelyhood' was
here
packet-acl.c: In function 'acl_message_likelyhood':
packet-acl.c:728:4: error: statement with no effect [-Werror=unused-value]
packet-acl.c:709:10: error: unused variable 'str' [-Werror=unused-variable]
packet-acl.c: At top level:
packet-acl.c:754:6: error: static declaration of 'acl_process_header' follows
non-static declaration
packet-acl.c:122:6: note: previous declaration of 'acl_process_header' was here
packet-acl.c:816:6: error: static declaration of 'acl_process_content' follows
non-static declaration
packet-acl.c:124:6: note: previous declaration of 'acl_process_content' was
here
packet-acl.c: In function 'acl_process_content':
packet-acl.c:855:22: error: request for implicit conversion from 'void *' to
'struct acl_marker_t *' not permitted in C++ [-Werror=c++-compat]
packet-acl.c:850:11: error: variable 'param_name' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:827:10: error: variable 'field_name' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:826:10: error: variable 'field' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:816:55: error: unused parameter 'pinfo' [-Werror=unused-parameter]
packet-acl.c: At top level:
packet-acl.c:887:6: error: static declaration of
'acl_dissector_add_tree_string' follows non-static declaration
packet-acl.c:127:6: note: previous declaration of
'acl_dissector_add_tree_string' was here
packet-acl.c: In function 'acl_dissector_add_tree_string':
packet-acl.c:898:5: error: format not a string literal and no format arguments
[-Werror=format-security]
packet-acl.c:891:15: error: variable 'tmp_item' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c: At top level:
packet-acl.c:924:6: error: static declaration of 'acl_parse_buffer' follows
non-static declaration
packet-acl.c:87:6: note: previous declaration of 'acl_parse_buffer' was here
packet-acl.c: In function 'acl_parse_buffer':
packet-acl.c:947:7: error: unused variable 'savedState'
[-Werror=unused-variable]
packet-acl.c:941:6: error: unused variable 'moredataneeded'
[-Werror=unused-variable]
packet-acl.c:940:10: error: unused variable 'tmp_tok' [-Werror=unused-variable]
packet-acl.c:938:6: error: variable 'h_end' set but not used
[-Werror=unused-but-set-variable]
packet-acl.c:937:6: error: unused variable 'h_start' [-Werror=unused-variable]
packet-acl.c: In function 'main':
packet-acl.c:1098:21: error: initialization discards 'const' qualifier from
pointer target type [-Werror]
packet-acl.c:1110:9: error: unused variable 'acl_ok_07'
[-Werror=unused-variable]
packet-acl.c:1107:9: error: unused variable 'acl_ok_06'
[-Werror=unused-variable]
packet-acl.c:1105:9: error: unused variable 'acl_ok_05'
[-Werror=unused-variable]
packet-acl.c:1103:9: error: unused variable 'acl_ok_04'
[-Werror=unused-variable]
packet-acl.c:1101:9: error: unused variable 'acl_ok_03'
[-Werror=unused-variable]
packet-acl.c:1099:9: error: unused variable 'acl_ok_02'
[-Werror=unused-variable]
packet-acl.c:1096:14: error: unused parameter 'argc' [-Werror=unused-parameter]
packet-acl.c:1096:26: error: unused parameter 'argv' [-Werror=unused-parameter]
packet-acl.c: In function 'test':
packet-acl.c:1132:10: error: unused variable 'offset' [-Werror=unused-variable]
packet-acl.c: At top level:
packet-acl.c:1156:15: error: static declaration of 'acl_get_status_name'
follows non-static declaration
packet-acl.c:91:7: note: previous declaration of 'acl_get_status_name' was here
packet-acl.c: In function 'acl_get_status_name':
packet-acl.c:1160:3: error: return discards 'const' qualifier from pointer
target type [-Werror]
packet-acl.c:1162:3: error: return discards 'const' qualifier from pointer
target type [-Werror]
packet-acl.c:1164:3: error: return discards 'const' qualifier from pointer
target type [-Werror]
packet-acl.c:1166:3: error: return discards 'const' qualifier from pointer
target type [-Werror]
packet-acl.c: At top level:
packet-acl.c:1170:15: error: static declaration of 'acl_get_state_name' follows
non-static declaration
packet-acl.c:92:7: note: previous declaration of 'acl_get_state_name' was here
packet-acl.c: In function 'acl_get_state_name':
packet-acl.c:1174:3: error: return discards 'const' qualifier from pointer
target type [-Werror]
packet-acl.c: At top level:
packet-acl.c:1196:17: error: static declaration of
'acl_is_known_acl_performative' follows non-static declaration
packet-acl.c:93:10: note: previous declaration of
'acl_is_known_acl_performative' was here
packet-acl.c:1209:17: error: static declaration of 'acl_is_known_acl_field'
follows non-static declaration
packet-acl.c:94:10: note: previous declaration of 'acl_is_known_acl_field' was
here
packet-acl.c:1222:15: error: static declaration of 'acl_marker_new' follows
non-static declaration
packet-acl.c:95:15: note: previous declaration of 'acl_marker_new' was here
packet-acl.c: In function 'acl_marker_new':
packet-acl.c:1225:22: error: request for implicit conversion from 'void *' to
'struct acl_marker_t *' not permitted in C++ [-Werror=c++-compat]
packet-acl.c: At top level:
packet-acl.c:1235:6: error: static declaration of 'acl_marker_list_add' follows
non-static declaration
packet-acl.c:98:6: note: previous declaration of 'acl_marker_list_add' was here
packet-acl.c:1244:6: error: static declaration of 'acl_marker_print' follows
non-static declaration
packet-acl.c:101:6: note: previous declaration of 'acl_marker_print' was here
packet-acl.c: In function 'acl_marker_print':
packet-acl.c:1244:30: error: declaration of 'log' shadows a global declaration
[-Werror=shadow]
packet-acl.c:183:34: error: shadowed declaration is here [-Werror=shadow]
packet-acl.c: At top level:
packet-acl.c:1270:6: error: static declaration of 'acl_marker_list_print'
follows non-static declaration
packet-acl.c:102:6: note: previous declaration of 'acl_marker_list_print' was
here
packet-acl.c: In function 'acl_marker_list_print':
packet-acl.c:1270:35: error: declaration of 'log' shadows a global declaration
[-Werror=shadow]
packet-acl.c:183:34: error: shadowed declaration is here [-Werror=shadow]
packet-acl.c:1281:4: error: request for implicit conversion from 'void *' to
'struct acl_marker_t *' not permitted in C++ [-Werror=c++-compat]
packet-acl.c:1272:8: error: unused variable 'i' [-Werror=unused-variable]
packet-acl.c: At top level:
packet-acl.c:185:34: error: 'global_enable_acl_logging' defined but not used
[-Werror=unused-variable]
packet-acl.c:323:6: error: 'acl_set_pinfo' defined but not used
[-Werror=unused-function]
packet-acl.c:340:6: error: 'acl_process_performative' defined but not used
[-Werror=unused-function]
packet-acl.c:386:6: error: 'acl_get_bounds' defined but not used
[-Werror=unused-function]
packet-acl.c:421:8: error: 'acl_str_rem_spaces' defined but not used
[-Werror=unused-function]
packet-acl.c:707:10: error: 'acl_message_likelyhood' defined but not used
[-Werror=unused-function]
packet-acl.c:754:6: error: 'acl_process_header' defined but not used
[-Werror=unused-function]
packet-acl.c:816:6: error: 'acl_process_content' defined but not used
[-Werror=unused-function]
packet-acl.c:887:6: error: 'acl_dissector_add_tree_string' defined but not used
[-Werror=unused-function]
packet-acl.c:1156:15: error: 'acl_get_status_name' defined but not used
[-Werror=unused-function]
packet-acl.c:1170:15: error: 'acl_get_state_name' defined but not used
[-Werror=unused-function]
packet-acl.c:1196:17: error: 'acl_is_known_acl_performative' defined but not
used [-Werror=unused-function]
packet-acl.c:1209:17: error: 'acl_is_known_acl_field' defined but not used
[-Werror=unused-function]
packet-acl.c:1235:6: error: 'acl_marker_list_add' defined but not used
[-Werror=unused-function]
packet-acl.c:1270:6: error: 'acl_marker_list_print' defined but not used
[-Werror=unused-function]

---

      You are receiving this mail because:

• You are watching all bug changes.