From 494508f2d02d8380c4060354fa06de6a3de417f4 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Sun, 25 Mar 2018 15:09:56 -0700 Subject: [PATCH] Clean up REPORT_DISSECTOR_BUG(). Have it take a format and argument list as arguments, and have the formatting done inside the reporting code. That way, we're not relying on any particular wmem scope working. If WIRESHARK_ABORT_ON_DISSECTOR_BUG is set, try to add the message to the crash information (currently only supported in macOS), and print it to the standard error, before crashing. We won't necessarily have a usable crash dump to analyze, so we can't rely on that to find the cause of the crash. Ping-Bug: 14490 Change-Id: I2b39169c45c84f2ada31efa1d413bd28c140f8f4 Reviewed-on: https://code.wireshark.org/review/26643 Petri-Dish: Guy Harris Reviewed-by: Guy Harris --- .../asn1/acse/packet-acse-template.c | 3 +- epan/dissectors/packet-acse.c | 7 +- epan/dissectors/packet-isakmp.c | 65 +++---- epan/dissectors/packet-lustre.c | 9 +- epan/except.c | 13 +- epan/except.h | 2 + epan/exceptions.h | 4 + epan/proto.c | 164 +++++++++--------- epan/proto.h | 67 +++---- wsutil/crash_info.c | 17 +- wsutil/crash_info.h | 2 + 11 files changed, 173 insertions(+), 180 deletions(-) diff --git a/epan/dissectors/asn1/acse/packet-acse-template.c b/epan/dissectors/asn1/acse/packet-acse-template.c index 73fe01f03c..d10208abb8 100644 --- a/epan/dissectors/asn1/acse/packet-acse-template.c +++ b/epan/dissectors/asn1/acse/packet-acse-template.c @@ -155,8 +155,7 @@ dissect_acse(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d session = ( (struct SESSION_DATA_STRUCTURE*)data); if (session->spdu_type == 0) { if (parent_tree) { - REPORT_DISSECTOR_BUG( - wmem_strdup_printf(wmem_packet_scope(), "Wrong spdu type %x from session dissector.",session->spdu_type)); + REPORT_DISSECTOR_BUG("Wrong spdu type %x from session dissector.",session->spdu_type); return 0; } } diff --git a/epan/dissectors/packet-acse.c b/epan/dissectors/packet-acse.c index db455c402a..4ed0c78fcb 100644 --- a/epan/dissectors/packet-acse.c +++ b/epan/dissectors/packet-acse.c @@ -1706,8 +1706,7 @@ dissect_acse(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d session = ( (struct SESSION_DATA_STRUCTURE*)data); if (session->spdu_type == 0) { if (parent_tree) { - REPORT_DISSECTOR_BUG( - wmem_strdup_printf(wmem_packet_scope(), "Wrong spdu type %x from session dissector.",session->spdu_type)); + REPORT_DISSECTOR_BUG("Wrong spdu type %x from session dissector.",session->spdu_type); return 0; } } @@ -2219,7 +2218,7 @@ void proto_register_acse(void) { NULL, HFILL }}, /*--- End of included file: packet-acse-hfarr.c ---*/ -#line 239 "./asn1/acse/packet-acse-template.c" +#line 238 "./asn1/acse/packet-acse-template.c" }; /* List of subtrees */ @@ -2265,7 +2264,7 @@ void proto_register_acse(void) { &ett_acse_Authentication_value, /*--- End of included file: packet-acse-ettarr.c ---*/ -#line 245 "./asn1/acse/packet-acse-template.c" +#line 244 "./asn1/acse/packet-acse-template.c" }; static ei_register_info ei[] = { diff --git a/epan/dissectors/packet-isakmp.c b/epan/dissectors/packet-isakmp.c index f3d9d575fb..cf3cec1bd2 100644 --- a/epan/dissectors/packet-isakmp.c +++ b/epan/dissectors/packet-isakmp.c @@ -5387,9 +5387,8 @@ dissect_enc(tvbuff_t *tvb, /* Check if encr/auth specs are set properly (if for some case not, wireshark would crash) */ if (!key_info->encr_spec || !key_info->auth_spec) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2: decryption/integrity specs not set-up properly: encr_spec: %p, auth_spec: %p", - (void *)key_info->auth_spec, (void*)key_info->auth_spec)); + REPORT_DISSECTOR_BUG("IKEv2: decryption/integrity specs not set-up properly: encr_spec: %p, auth_spec: %p", + (void *)key_info->auth_spec, (void*)key_info->auth_spec); } iv_len = key_info->encr_spec->iv_len; @@ -5459,16 +5458,14 @@ dissect_enc(tvbuff_t *tvb, proto_item_append_text(icd_item, " <%s>", val_to_str(key_info->auth_spec->number, vs_ikev2_auth_algs, "Unknown mac algo: %d")); err = gcry_md_open(&md_hd, key_info->auth_spec->gcry_alg, key_info->auth_spec->gcry_flag); if (err) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 hashing error: algorithm %d: gcry_md_open failed: %s", - key_info->auth_spec->gcry_alg, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 hashing error: algorithm %d: gcry_md_open failed: %s", + key_info->auth_spec->gcry_alg, gcry_strerror(err)); } err = gcry_md_setkey(md_hd, key_info->auth_key, key_info->auth_spec->key_len); if (err) { gcry_md_close(md_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 hashing error: algorithm %s, key length %u: gcry_md_setkey failed: %s", - gcry_md_algo_name(key_info->auth_spec->gcry_alg), key_info->auth_spec->key_len, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 hashing error: algorithm %s, key length %u: gcry_md_setkey failed: %s", + gcry_md_algo_name(key_info->auth_spec->gcry_alg), key_info->auth_spec->key_len, gcry_strerror(err)); } /* Calculate hash over the bytes from the beginning of the ISAKMP header to the right before the ICD. */ @@ -5478,9 +5475,8 @@ dissect_enc(tvbuff_t *tvb, md_len = gcry_md_get_algo_dlen(key_info->auth_spec->gcry_alg); if (md_len < icd_len) { gcry_md_close(md_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 hashing error: algorithm %s: gcry_md_get_algo_dlen returned %d which is smaller than icd length %d", - gcry_md_algo_name(key_info->auth_spec->gcry_alg), md_len, icd_len)); + REPORT_DISSECTOR_BUG("IKEv2 hashing error: algorithm %s: gcry_md_get_algo_dlen returned %d which is smaller than icd length %d", + gcry_md_algo_name(key_info->auth_spec->gcry_alg), md_len, icd_len); } if (tvb_memeql(tvb, offset, md, icd_len) == 0) { proto_item_append_text(icd_item, "[correct]"); @@ -5519,9 +5515,8 @@ dissect_enc(tvbuff_t *tvb, } else { err = gcry_cipher_open(&cipher_hd, key_info->encr_spec->gcry_alg, key_info->encr_spec->gcry_mode, 0); if (err) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d, mode %d: gcry_cipher_open failed: %s", - key_info->encr_spec->gcry_alg, key_info->encr_spec->gcry_mode, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d, mode %d: gcry_cipher_open failed: %s", + key_info->encr_spec->gcry_alg, key_info->encr_spec->gcry_mode, gcry_strerror(err)); } /* Handling CTR mode and AEAD ciphers */ @@ -5537,9 +5532,8 @@ dissect_enc(tvbuff_t *tvb, if (encr_key_len < 0 || encr_iv_len < encr_iv_offset + (int)key_info->encr_spec->salt_len + iv_len) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d, key length %d, salt length %d, input iv length %d, cipher iv length: %d: invalid length(s) of cipher parameters", - key_info->encr_spec->gcry_alg, encr_key_len, key_info->encr_spec->salt_len, iv_len, encr_iv_len)); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d, key length %d, salt length %d, input iv length %d, cipher iv length: %d: invalid length(s) of cipher parameters", + key_info->encr_spec->gcry_alg, encr_key_len, key_info->encr_spec->salt_len, iv_len, encr_iv_len); } encr_iv = (guchar *)wmem_alloc0(wmem_packet_scope(), encr_iv_len); @@ -5559,18 +5553,16 @@ dissect_enc(tvbuff_t *tvb, err = gcry_cipher_setkey(cipher_hd, key_info->encr_key, encr_key_len); if (err) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d, key length %d: gcry_cipher_setkey failed: %s", - key_info->encr_spec->gcry_alg, encr_key_len, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d, key length %d: gcry_cipher_setkey failed: %s", + key_info->encr_spec->gcry_alg, encr_key_len, gcry_strerror(err)); } if (key_info->encr_spec->gcry_mode == GCRY_CIPHER_MODE_CTR) err = gcry_cipher_setctr(cipher_hd, encr_iv, encr_iv_len); else err = gcry_cipher_setiv(cipher_hd, encr_iv, encr_iv_len); if (err) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d, iv length %d: gcry_cipher_setiv/gcry_cipher_setctr failed: %s", - key_info->encr_spec->gcry_alg, encr_iv_len, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d, iv length %d: gcry_cipher_setiv/gcry_cipher_setctr failed: %s", + key_info->encr_spec->gcry_alg, encr_iv_len, gcry_strerror(err)); } #ifdef HAVE_LIBGCRYPT_AEAD @@ -5583,9 +5575,8 @@ dissect_enc(tvbuff_t *tvb, err = gcry_cipher_ctl(cipher_hd, GCRYCTL_SET_CCM_LENGTHS, ccm_lengths, sizeof(ccm_lengths)); if (err) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d: gcry_cipher_ctl(GCRYCTL_SET_CCM_LENGTHS) failed: %s", - key_info->encr_spec->gcry_alg, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d: gcry_cipher_ctl(GCRYCTL_SET_CCM_LENGTHS) failed: %s", + key_info->encr_spec->gcry_alg, gcry_strerror(err)); } } @@ -5593,9 +5584,8 @@ dissect_enc(tvbuff_t *tvb, err = gcry_cipher_authenticate(cipher_hd, aa_data, aad_len); if (err) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d: gcry_cipher_authenticate failed: %s", - key_info->encr_spec->gcry_alg, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d: gcry_cipher_authenticate failed: %s", + key_info->encr_spec->gcry_alg, gcry_strerror(err)); } } #endif @@ -5603,9 +5593,8 @@ dissect_enc(tvbuff_t *tvb, err = gcry_cipher_decrypt(cipher_hd, decr_data, decr_data_len, encr_data, encr_data_len); if (err) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d: gcry_cipher_decrypt failed: %s", - key_info->encr_spec->gcry_alg, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d: gcry_cipher_decrypt failed: %s", + key_info->encr_spec->gcry_alg, gcry_strerror(err)); } #ifdef HAVE_LIBGCRYPT_AEAD @@ -5632,18 +5621,16 @@ dissect_enc(tvbuff_t *tvb, if (tag_len < icv_len) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d: gcry_cipher_get_algo_blklen returned %d which is smaller than icv length %d", - key_info->encr_spec->gcry_alg, tag_len, icv_len)); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d: gcry_cipher_get_algo_blklen returned %d which is smaller than icv length %d", + key_info->encr_spec->gcry_alg, tag_len, icv_len); } tag = (guchar *)wmem_alloc(wmem_packet_scope(), tag_len); err = gcry_cipher_gettag(cipher_hd, tag, tag_len); if (err) { gcry_cipher_close(cipher_hd); - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "IKEv2 decryption error: algorithm %d: gcry_cipher_gettag failed: %s", - key_info->encr_spec->gcry_alg, gcry_strerror(err))); + REPORT_DISSECTOR_BUG("IKEv2 decryption error: algorithm %d: gcry_cipher_gettag failed: %s", + key_info->encr_spec->gcry_alg, gcry_strerror(err)); } else if (memcmp(tag, icv_data, icv_len) == 0) proto_item_append_text(icd_item, "[correct]"); diff --git a/epan/dissectors/packet-lustre.c b/epan/dissectors/packet-lustre.c index 60ae5e89e3..ecc8d9fac8 100644 --- a/epan/dissectors/packet-lustre.c +++ b/epan/dissectors/packet-lustre.c @@ -1581,11 +1581,10 @@ lustre_get_trans(packet_info *pinfo, struct lnet_trans_info *info) if (ptr != NULL) { /* XXX - Is this even possible? ?*/ trans = (lustre_trans_t *)ptr; - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "ERROR: packet-lustre: conversation replaced: " - "trans:{opcode:%u sub_opcode:%" G_GINT64_MODIFIER "u match_bits:%" G_GINT64_MODIFIER "x} " - "with match_bits:%" G_GINT64_MODIFIER "x", - trans->opcode, trans->sub_opcode, trans->match_bits, info->match_bits)); + REPORT_DISSECTOR_BUG("ERROR: packet-lustre: conversation replaced: " + "trans:{opcode:%u sub_opcode:%" G_GINT64_MODIFIER "u match_bits:%" G_GINT64_MODIFIER "x} " + "with match_bits:%" G_GINT64_MODIFIER "x", + trans->opcode, trans->sub_opcode, trans->match_bits, info->match_bits); } } diff --git a/epan/except.c b/epan/except.c index 2420b33760..dc95301d51 100644 --- a/epan/except.c +++ b/epan/except.c @@ -309,15 +309,22 @@ WS_NORETURN void except_throwd(long group, long code, const char *msg, void *dat * XCEPT_BUFFER_SIZE? We could then just use this to generate formatted * messages. */ -WS_NORETURN void except_throwf(long group, long code, const char *fmt, ...) +WS_NORETURN void except_vthrowf(long group, long code, const char *fmt, + va_list vl) { char *buf = (char *)except_alloc(XCEPT_BUFFER_SIZE); + + g_vsnprintf(buf, XCEPT_BUFFER_SIZE, fmt, vl); + except_throwd(group, code, buf, buf); +} + +WS_NORETURN void except_throwf(long group, long code, const char *fmt, ...) +{ va_list vl; va_start (vl, fmt); - g_vsnprintf(buf, XCEPT_BUFFER_SIZE, fmt, vl); + except_vthrowf(group, code, fmt, vl); va_end (vl); - except_throwd(group, code, buf, buf); } void (*except_unhandled_catcher(void (*new_catcher)(except_t *)))(except_t *) diff --git a/epan/except.h b/epan/except.h index 45ce069c72..fdb33d8f03 100644 --- a/epan/except.h +++ b/epan/except.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include "ws_symbol_export.h" #include "ws_attributes.h" @@ -96,6 +97,7 @@ WS_DLL_PUBLIC void except_deinit(void); WS_DLL_PUBLIC WS_NORETURN void except_rethrow(except_t *); WS_DLL_PUBLIC WS_NORETURN void except_throw(long, long, const char *); WS_DLL_PUBLIC WS_NORETURN void except_throwd(long, long, const char *, void *); +WS_DLL_PUBLIC WS_NORETURN void except_vthrowf(long group, long code, const char *fmt, va_list vl); WS_DLL_PUBLIC WS_NORETURN void except_throwf(long, long, const char *, ...) G_GNUC_PRINTF(3, 4); WS_DLL_PUBLIC void (*except_unhandled_catcher(void (*)(except_t *)))(except_t *); diff --git a/epan/exceptions.h b/epan/exceptions.h index baf64889f8..5e3bedf32f 100644 --- a/epan/exceptions.h +++ b/epan/exceptions.h @@ -364,6 +364,10 @@ #define THROW_FORMATTED(x, ...) \ except_throwf(XCEPT_GROUP_WIRESHARK, (x), __VA_ARGS__) +/* Like THROW_FORMATTED, but takes a va_list as an argument */ +#define VTHROW_FORMATTED(x, format, args) \ + except_vthrowf(XCEPT_GROUP_WIRESHARK, (x), format, args) + #define GET_MESSAGE except_message(exc) #define RETHROW \ diff --git a/epan/proto.c b/epan/proto.c index 0d3f5cf6da..8720586241 100644 --- a/epan/proto.c +++ b/epan/proto.c @@ -44,6 +44,7 @@ #include "in_cksum.h" #include /* ws_debug_printf/ws_g_warning */ +#include #include /* Ptvcursor limits */ @@ -1322,12 +1323,36 @@ proto_tree_add_format_wsp_text(proto_tree *tree, tvbuff_t *tvb, gint start, gint return pi; } -void proto_report_dissector_bug(const char *message) +void proto_report_dissector_bug(const char *format, ...) { - if (getenv("WIRESHARK_ABORT_ON_DISSECTOR_BUG") != NULL) + va_list args; + + if (getenv("WIRESHARK_ABORT_ON_DISSECTOR_BUG") != NULL) { + /* + * Try to have the error message show up in the crash + * information. + */ + va_start(args, format); + ws_vadd_crash_info(format, args); + va_end(args); + + /* + * Print the error message. + */ + va_start(args, format); + vfprintf(stderr, format, args); + va_end(args); + putc('\n', stderr); + + /* + * And crash. + */ abort(); - else - THROW_MESSAGE(DissectorError, message); + } else { + va_start(args, format); + VTHROW_FORMATTED(DissectorError, format, args); + va_end(args); + } } /* We could probably get away with changing is_error to a minimum length value. */ @@ -2552,9 +2577,8 @@ proto_tree_add_item_ret_int(proto_tree *tree, int hfindex, tvbuff_t *tvb, /* length validation for native number encoding caught by get_uint_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_int", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_int", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -2607,17 +2631,15 @@ proto_tree_add_item_ret_uint(proto_tree *tree, int hfindex, tvbuff_t *tvb, case FT_UINT32: break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", + hfinfo->abbrev); } /* length validation for native number encoding caught by get_uint_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_uint", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_uint", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -2680,9 +2702,8 @@ ptvcursor_add_ret_uint(ptvcursor_t *ptvc, int hfindex, gint length, case FT_UINT32: break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", + hfinfo->abbrev); } get_hfi_length(hfinfo, ptvc->tvb, offset, &length, &item_length, encoding); @@ -2738,9 +2759,8 @@ ptvcursor_add_ret_int(ptvcursor_t *ptvc, int hfindex, gint length, case FT_INT32: break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, or FT_UINT32", + hfinfo->abbrev); } get_hfi_length(hfinfo, ptvc->tvb, offset, &length, &item_length, encoding); @@ -2806,9 +2826,8 @@ ptvcursor_add_ret_string(ptvcursor_t* ptvc, int hf, gint length, const guint enc value = get_stringzpad_value(scope, ptvc->tvb, offset, length, &item_length, encoding); break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_STRING, FT_STRINGZ, FT_UINT_STRING, or FT_STRINGZPAD", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_STRING, FT_STRINGZ, FT_UINT_STRING, or FT_STRINGZPAD", + hfinfo->abbrev); } if (retval) @@ -2841,16 +2860,15 @@ ptvcursor_add_ret_boolean(ptvcursor_t* ptvc, int hfindex, gint length, const gui PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo); if (hfinfo->type != FT_BOOLEAN) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_BOOLEAN", hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_BOOLEAN", + hfinfo->abbrev); } /* length validation for native number encoding caught by get_uint64_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_uint", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_uint", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -2895,16 +2913,15 @@ proto_tree_add_item_ret_uint64(proto_tree *tree, int hfindex, tvbuff_t *tvb, DISSECTOR_ASSERT_HINT(hfinfo != NULL, "Not passed hfi!"); if (hfinfo->type != FT_UINT64) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_UINT64", hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_UINT64", + hfinfo->abbrev); } /* length validation for native number encoding caught by get_uint64_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_uint", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_uint", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -2953,16 +2970,15 @@ proto_tree_add_item_ret_varint(proto_tree *tree, int hfindex, tvbuff_t *tvb, DISSECTOR_ASSERT_HINT(hfinfo != NULL, "Not passed hfi!"); if ((!IS_FT_INT(hfinfo->type)) && (!IS_FT_UINT(hfinfo->type))) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_UINT or FT_INT", hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_UINT or FT_INT", + hfinfo->abbrev); } /* length validation for native number encoding caught by get_uint64_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_varint", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_varint", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -3013,16 +3029,15 @@ proto_tree_add_item_ret_boolean(proto_tree *tree, int hfindex, tvbuff_t *tvb, DISSECTOR_ASSERT_HINT(hfinfo != NULL, "Not passed hfi!"); if (hfinfo->type != FT_BOOLEAN) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_BOOLEAN", hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_BOOLEAN", + hfinfo->abbrev); } /* length validation for native number encoding caught by get_uint64_value() */ /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_item_ret_uint", - length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_item_ret_uint", + length); if (encoding & ENC_STRING) { REPORT_DISSECTOR_BUG("wrong encoding"); @@ -3081,9 +3096,8 @@ proto_tree_add_item_ret_string_and_length(proto_tree *tree, int hfindex, value = get_stringzpad_value(scope, tvb, start, length, lenretval, encoding); break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_STRING, FT_STRINGZ, FT_UINT_STRING, or FT_STRINGZPAD", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_STRING, FT_STRINGZ, FT_UINT_STRING, or FT_STRINGZPAD", + hfinfo->abbrev); } if (retval) @@ -3271,9 +3285,8 @@ proto_tree_add_bytes_item(proto_tree *tree, int hfindex, tvbuff_t *tvb, /* length has to be -1 or > 0 regardless of encoding */ /* invalid FT_UINT_BYTES length is caught in get_uint_value() */ if (length < -1 || length == 0) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_bytes_item for %s", - length, ftype_name(hfinfo->type))); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_bytes_item for %s", + length, ftype_name(hfinfo->type)); } if (encoding & ENC_STR_NUM) { @@ -3387,8 +3400,8 @@ proto_tree_add_time_item(proto_tree *tree, int hfindex, tvbuff_t *tvb, /* length has to be -1 or > 0 regardless of encoding */ if (length < -1 || length == 0) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Invalid length %d passed to proto_tree_add_time_item", length)); + REPORT_DISSECTOR_BUG("Invalid length %d passed to proto_tree_add_time_item", + length); } time_stamp.secs = 0; @@ -4626,9 +4639,8 @@ proto_tree_add_uint(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start, break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, FT_UINT32, or FT_FRAMENUM", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24, FT_UINT32, or FT_FRAMENUM", + hfinfo->abbrev); } return pi; @@ -4716,9 +4728,8 @@ proto_tree_add_uint64(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start, break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_UINT40, FT_UINT48, FT_UINT56, FT_UINT64, or FT_FRAMENUM", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_UINT40, FT_UINT48, FT_UINT56, FT_UINT64, or FT_FRAMENUM", + hfinfo->abbrev); } return pi; @@ -4805,9 +4816,8 @@ proto_tree_add_int(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start, break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_INT8, FT_INT16, FT_INT24, or FT_INT32", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_INT8, FT_INT16, FT_INT24, or FT_INT32", + hfinfo->abbrev); } return pi; @@ -4898,9 +4908,8 @@ proto_tree_add_int64(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start, break; default: - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is not of type FT_INT40, FT_INT48, FT_INT56, or FT_INT64", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is not of type FT_INT40, FT_INT48, FT_INT56, or FT_INT64", + hfinfo->abbrev); } return pi; @@ -5081,9 +5090,8 @@ proto_tree_add_node(proto_tree *tree, field_info *fi) tnode = tree; tfi = PNODE_FINFO(tnode); if (tfi != NULL && (tfi->tree_type < 0 || tfi->tree_type >= num_tree_types)) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "\"%s\" - \"%s\" tfi->tree_type: %d invalid (%s:%u)", - fi->hfinfo->name, fi->hfinfo->abbrev, tfi->tree_type, __FILE__, __LINE__)); + REPORT_DISSECTOR_BUG("\"%s\" - \"%s\" tfi->tree_type: %d invalid (%s:%u)", + fi->hfinfo->name, fi->hfinfo->abbrev, tfi->tree_type, __FILE__, __LINE__); /* XXX - is it safe to continue here? */ } @@ -8488,9 +8496,8 @@ hf_try_val64_to_str(guint64 value, const header_field_info *hfinfo) /* If this is reached somebody registered a 64-bit field with a 32-bit * value-string, which isn't right. */ - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "field %s is a 64-bit field with a 32-bit value_string", - hfinfo->abbrev)); + REPORT_DISSECTOR_BUG("field %s is a 64-bit field with a 32-bit value_string", + hfinfo->abbrev); /* This is necessary to squelch MSVC errors; is there any way to tell it that DISSECTOR_ASSERT_NOT_REACHED() @@ -11021,10 +11028,9 @@ _proto_tree_add_bits_ret_val(proto_tree *tree, const int hfindex, tvbuff_t *tvb, PROTO_REGISTRAR_GET_NTH(hfindex, hf_field); if (hf_field->bitmask != 0) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Incompatible use of proto_tree_add_bits_ret_val" - " with field '%s' (%s) with bitmask != 0", - hf_field->abbrev, hf_field->name)); + REPORT_DISSECTOR_BUG("Incompatible use of proto_tree_add_bits_ret_val" + " with field '%s' (%s) with bitmask != 0", + hf_field->abbrev, hf_field->name); } DISSECTOR_ASSERT(no_of_bits > 0); @@ -11156,10 +11162,9 @@ proto_tree_add_split_bits_item_ret_val(proto_tree *tree, const int hfindex, tvbu PROTO_REGISTRAR_GET_NTH(hfindex, hf_field); if (hf_field->bitmask != 0) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Incompatible use of proto_tree_add_split_bits_item_ret_val" - " with field '%s' (%s) with bitmask != 0", - hf_field->abbrev, hf_field->name)); + REPORT_DISSECTOR_BUG("Incompatible use of proto_tree_add_split_bits_item_ret_val" + " with field '%s' (%s) with bitmask != 0", + hf_field->abbrev, hf_field->name); } mask_initial_bit_offset = bit_offset % 8; @@ -11359,10 +11364,9 @@ _proto_tree_add_bits_format_value(proto_tree *tree, const int hfindex, TRY_TO_FAKE_THIS_ITEM(tree, hfindex, hf_field); if (hf_field->bitmask != 0) { - REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(), - "Incompatible use of proto_tree_add_bits_format_value" - " with field '%s' (%s) with bitmask != 0", - hf_field->abbrev, hf_field->name)); + REPORT_DISSECTOR_BUG("Incompatible use of proto_tree_add_bits_format_value" + " with field '%s' (%s) with bitmask != 0", + hf_field->abbrev, hf_field->name); } DISSECTOR_ASSERT(no_of_bits > 0); diff --git a/epan/proto.h b/epan/proto.h index bb7d1f8f7e..96e88c439e 100644 --- a/epan/proto.h +++ b/epan/proto.h @@ -104,22 +104,21 @@ struct _protocol; typedef struct _protocol protocol_t; /** Function used for reporting errors in dissectors; it throws a - * DissectorError exception, with the string passed as an argument - * as the message for the exception, so that it can show up in - * the Info column and the protocol tree. - * - * If that string is dynamically allocated, it should be allocated with - * wmem_alloc() with wmem_packet_scope(); using wmem_strdup_printf() would work. + * DissectorError exception, with a string generated from the format + * and arguments to the format, as the message for the exception, so + * that it can show up in the Info column and the protocol tree. * * If the WIRESHARK_ABORT_ON_DISSECTOR_BUG environment variable is set, * it will call abort(), instead, to make it easier to get a stack trace. * - * @param message string to use as the message + * @param format format string to use for the message */ -WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); +WS_DLL_PUBLIC WS_NORETURN +void proto_report_dissector_bug(const char *format, ...) + G_GNUC_PRINTF(1, 2); -#define REPORT_DISSECTOR_BUG(message) \ - proto_report_dissector_bug(message) +#define REPORT_DISSECTOR_BUG(...) \ + proto_report_dissector_bug(__VA_ARGS__) /** Macro used to provide a hint to static analysis tools. * (Currently only Visual C++.) @@ -146,16 +145,12 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); #define __DISSECTOR_ASSERT_STRINGIFY(s) # s #define __DISSECTOR_ASSERT(expression, file, lineno) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: failed assertion \"%s\"", \ - file, lineno, __DISSECTOR_ASSERT_STRINGIFY(expression)))) + (REPORT_DISSECTOR_BUG("%s:%u: failed assertion \"%s\"", \ + file, lineno, __DISSECTOR_ASSERT_STRINGIFY(expression))) #define __DISSECTOR_ASSERT_HINT(expression, file, lineno, hint) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: failed assertion \"%s\" (%s)", \ - file, lineno, __DISSECTOR_ASSERT_STRINGIFY(expression), hint))) + (REPORT_DISSECTOR_BUG("%s:%u: failed assertion \"%s\" (%s)", \ + file, lineno, __DISSECTOR_ASSERT_STRINGIFY(expression), hint)) #define DISSECTOR_ASSERT(expression) \ ((void) ((expression) ? (void)0 : \ @@ -190,10 +185,8 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); * */ #define DISSECTOR_ASSERT_NOT_REACHED() \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: failed assertion \"DISSECTOR_ASSERT_NOT_REACHED\"", \ - __FILE__, __LINE__))) + (REPORT_DISSECTOR_BUG("%s:%u: failed assertion \"DISSECTOR_ASSERT_NOT_REACHED\"", \ + __FILE__, __LINE__)) /** Compare two integers. * @@ -215,10 +208,8 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); * @param fmt the fmt operator */ #define __DISSECTOR_ASSERT_CMPINT(a, op, b, type, fmt) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: failed assertion " #a " " #op " " #b " (" fmt " " #op " " fmt ")", \ - __FILE__, __LINE__, (type)a, (type)b))) + (REPORT_DISSECTOR_BUG("%s:%u: failed assertion " #a " " #op " " #b " (" fmt " " #op " " fmt ")", \ + __FILE__, __LINE__, (type)a, (type)b)) #define DISSECTOR_ASSERT_CMPINT(a, op, b) \ ((void) ((a op b) ? (void)0 : \ @@ -252,10 +243,8 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); * @param type The type it's expected to have */ #define __DISSECTOR_ASSERT_FIELD_TYPE(hfinfo, t) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: field %s is not of type "#t, \ - __FILE__, __LINE__, (hfinfo)->abbrev))) + (REPORT_DISSECTOR_BUG("%s:%u: field %s is not of type "#t, \ + __FILE__, __LINE__, (hfinfo)->abbrev)) #define DISSECTOR_ASSERT_FIELD_TYPE(hfinfo, t) \ ((void) (((hfinfo)->type == t) ? (void)0 : \ @@ -265,18 +254,14 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); #define DISSECTOR_ASSERT_FIELD_TYPE_IS_INTEGRAL(hfinfo) \ ((void) ((IS_FT_INT((hfinfo)->type) || \ IS_FT_UINT((hfinfo)->type)) ? (void)0 : \ - REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: field %s is not of type FT_CHAR or an FT_{U}INTn type", \ - __FILE__, __LINE__, (hfinfo)->abbrev)))) \ + REPORT_DISSECTOR_BUG("%s:%u: field %s is not of type FT_CHAR or an FT_{U}INTn type", \ + __FILE__, __LINE__, (hfinfo)->abbrev))) \ __DISSECTOR_ASSERT_STATIC_ANALYSIS_HINT(IS_FT_INT((hfinfo)->type) || \ IS_FT_UINT((hfinfo)->type)) #define __DISSECTOR_ASSERT_FIELD_TYPE_IS_STRING(hfinfo) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: field %s is not of type FT_STRING, FT_STRINGZ, or FT_STRINGZPAD", \ - __FILE__, __LINE__, (hfinfo)->abbrev))) + (REPORT_DISSECTOR_BUG("%s:%u: field %s is not of type FT_STRING, FT_STRINGZ, or FT_STRINGZPAD", \ + __FILE__, __LINE__, (hfinfo)->abbrev)) #define DISSECTOR_ASSERT_FIELD_TYPE_IS_STRING(hfinfo) \ ((void) (((hfinfo)->type == FT_STRING || (hfinfo)->type == FT_STRINGZ || \ @@ -287,10 +272,8 @@ WS_DLL_PUBLIC WS_NORETURN void proto_report_dissector_bug(const char *message); (hfinfo)->type == FT_STRINGZPAD) #define __DISSECTOR_ASSERT_FIELD_TYPE_IS_TIME(hfinfo) \ - (REPORT_DISSECTOR_BUG( \ - wmem_strdup_printf(wmem_packet_scope(), \ - "%s:%u: field %s is not of type FT_ABSOLUTE_TIME or FT_RELATIVE_TIME", \ - __FILE__, __LINE__, (hfinfo)->abbrev))) + (REPORT_DISSECTOR_BUG("%s:%u: field %s is not of type FT_ABSOLUTE_TIME or FT_RELATIVE_TIME", \ + __FILE__, __LINE__, (hfinfo)->abbrev)) #define DISSECTOR_ASSERT_FIELD_TYPE_IS_TIME(hfinfo) \ ((void) (((hfinfo)->type == FT_ABSOLUTE_TIME || \ diff --git a/wsutil/crash_info.c b/wsutil/crash_info.c index 7f13513267..afb9f921ca 100644 --- a/wsutil/crash_info.c +++ b/wsutil/crash_info.c @@ -127,14 +127,11 @@ struct crashreporter_annotations_t gCRAnnotations #endif /* 0 */ void -ws_add_crash_info(const char *fmt, ...) +ws_vadd_crash_info(const char *fmt, va_list ap) { - va_list ap; char *m, *old_info, *new_info; - va_start(ap, fmt); m = g_strdup_vprintf(fmt, ap); - va_end(ap); if (__crashreporter_info__ == NULL) __crashreporter_info__ = m; else { @@ -154,11 +151,21 @@ ws_add_crash_info(const char *fmt, ...) * ? */ void -ws_add_crash_info(const char *fmt _U_, ...) +ws_vadd_crash_info(const char *fmt _U_, va_list ap _U_) { } #endif /* __APPLE__ */ +void +ws_add_crash_info(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + ws_vadd_crash_info(fmt, ap); + va_end(ap); +} + /* * Editor modelines - http://www.wireshark.org/tools/modelines.html * diff --git a/wsutil/crash_info.h b/wsutil/crash_info.h index 8cb9cd3a21..ec50df345b 100644 --- a/wsutil/crash_info.h +++ b/wsutil/crash_info.h @@ -18,6 +18,8 @@ extern "C" { #endif +WS_DLL_PUBLIC void ws_vadd_crash_info(const char *fmt, va_list ap); + WS_DLL_PUBLIC void ws_add_crash_info(const char *fmt, ...) G_GNUC_PRINTF(1,2);