diff options
author | Guy Harris <guy@alum.mit.edu> | 2016-12-08 12:34:59 -0800 |
---|---|---|
committer | Guy Harris <guy@alum.mit.edu> | 2016-12-08 20:35:36 +0000 |
commit | d438170c87c429af914dbc66dd989860dd461960 (patch) | |
tree | 744940ebff7fcc2343fe46c70e4d3cc58e8b4994 /epan/dissectors/packet-cops.c | |
parent | a02d8e3c4e8cf7d756141ca45f6b27f5658747fb (diff) |
Fix a mis-merging.
Also, remove the "make sure we're not fetching a bogus structure" tests.
Add a comment explaining how a compiler bug where it's overly optimizing
a combination of tests could cause the valgrind errors we were seeing,
so we're zeroing the entire structure, padding included, to avoid that.
Change-Id: I24f94b2cbceec5234c1da82b891f609648075839
Reviewed-on: https://code.wireshark.org/review/19149
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Diffstat (limited to 'epan/dissectors/packet-cops.c')
-rw-r--r-- | epan/dissectors/packet-cops.c | 38 |
1 files changed, 22 insertions, 16 deletions
diff --git a/epan/dissectors/packet-cops.c b/epan/dissectors/packet-cops.c index f441402888..a23d61e91f 100644 --- a/epan/dissectors/packet-cops.c +++ b/epan/dissectors/packet-cops.c @@ -805,9 +805,6 @@ typedef struct _cops_conv_info_t { typedef struct _cops_call_t { -#if 1 -guint32 magic; -#endif guint8 op_code; gboolean solicited; guint32 req_num; @@ -1055,11 +1052,29 @@ dissect_cops_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data } if (!pinfo->fd->flags.visited) { + /* + * XXX - yes, we're setting all the fields in this + * structure, but there's padding between op_code + * and solicited, and that can't be set. + * + * For some reason, on some platforms, valgrind is + * complaining about a test of the solicited field + * accessing uninitialized data, perhaps because + * the 8 bytes containing op_code and solicited is + * being loaded as a unit. If the compiler is, for + * example, turning a test of + * + * cops_call->op_code == COPS_MSG_KA && !(cops_call->solicited) + * + * into a load of those 8 bytes and a comparison against a value + * with op_code being COPS_MSG_KA, solicited being false (0), + * *and* the padding being zero, it's buggy, but overly-"clever" + * buggy compilers do exist, so....) + * + * So we use wmem_new0() to forcibly zero out the entire + * structure before filling it in. + */ cops_call = wmem_new0(wmem_file_scope(), cops_call_t); - cops_call = wmem_new(wmem_file_scope(), cops_call_t); -#if 1 -cops_call->magic = 0xbeeff00d; -#endif cops_call->op_code = op_code; cops_call->solicited = is_solicited; cops_call->req_num = pinfo->num; @@ -1070,9 +1085,6 @@ cops_call->magic = 0xbeeff00d; else { for (i=0; i < pdus_array->len; i++) { cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i); -#if 1 -DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d); -#endif if ( cops_call->req_num == pinfo->num && cops_call->rsp_num != 0) { ti = proto_tree_add_uint_format(cops_tree, hf_cops_response_in, tvb, 0, 0, cops_call->rsp_num, @@ -1092,9 +1104,6 @@ DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d); if (!pinfo->fd->flags.visited) { for (i=0; i < pdus_array->len; i++) { cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i); -#if 1 -DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d); -#endif if (nstime_cmp(&pinfo->abs_ts, &cops_call->req_time) <= 0 || cops_call->rsp_num != 0) continue; @@ -1122,9 +1131,6 @@ DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d); else { for (i=0; i < pdus_array->len; i++) { cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i); -#if 1 -DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d); -#endif if ( cops_call->rsp_num == pinfo->num ) { ti = proto_tree_add_uint_format(cops_tree, hf_cops_response_to, tvb, 0, 0, cops_call->req_num, "Response to a request in frame %u", cops_call->req_num); |