From a4b7135ce3fa8362edf75804da448a362311e1b8 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Sat, 12 Dec 2009 03:15:28 +0000 Subject: [PATCH] Rename BASE_STRUCTURE_RESET to BASE_DISPLAY_E_MASK, to clarify that it's a mask to select the base_display_e value from a display field in a header_field_info structure. Never select that value by masking out the BASE_RANGE_STRING flag bit, as that won't continue to work if more flag bits, or other bitfields, are added. Instead, mask with BASE_DISPLAY_E_MASK. Note that the base_display_e value and BASE_RANGE_STRING flag are only for integral field types, and clarify what BASE_DISPLAY_E_MASK is. Give at least one of the reasons why hiding protocol fields is not considered a good idea. svn path=/trunk/; revision=31249 --- epan/proto.c | 22 ++++++++++------------ epan/proto.h | 32 ++++++++++++++++++++------------ gtk/dfilter_expr_dlg.c | 2 +- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/epan/proto.c b/epan/proto.c index af01ce2959..6256435af8 100644 --- a/epan/proto.c +++ b/epan/proto.c @@ -4815,9 +4815,8 @@ hfinfo_uint_vals_format(header_field_info *hfinfo) { const char *format = NULL; - /* bit operation to reset the potential BASE_RANGE_STRING (or others in - * the future?) */ - switch(hfinfo->display & BASE_STRUCTURE_RESET) { + /* Get the underlying BASE_ value */ + switch(hfinfo->display & BASE_DISPLAY_E_MASK) { case BASE_NONE: format = "%s: %s"; break; @@ -5028,9 +5027,8 @@ hfinfo_int_vals_format(header_field_info *hfinfo) { const char *format = NULL; - /* bit operation to reset the potential BASE_RANGE_STRING (or others in - * the future?)*/ - switch(hfinfo->display & BASE_STRUCTURE_RESET) { + /* Get the underlying BASE_ value */ + switch(hfinfo->display & BASE_DISPLAY_E_MASK) { case BASE_NONE: format = "%s: %s"; break; @@ -5600,7 +5598,7 @@ proto_registrar_dump_values(void) range = NULL; tfs = NULL; - if ((hfinfo->display & BASE_STRUCTURE_RESET) != BASE_CUSTOM && + if ((hfinfo->display & BASE_DISPLAY_E_MASK) != BASE_CUSTOM && (hfinfo->type == FT_UINT8 || hfinfo->type == FT_UINT16 || hfinfo->type == FT_UINT24 || @@ -5648,7 +5646,7 @@ proto_registrar_dump_values(void) vi = 0; while (range[vi].strptr) { /* Print in the proper base */ - if ((hfinfo->display & BASE_STRUCTURE_RESET) == BASE_HEX) { + if ((hfinfo->display & BASE_DISPLAY_E_MASK) == BASE_HEX) { printf("R\t%s\t0x%x\t0x%x\t%s\n", hfinfo->abbrev, range[vi].value_min, @@ -5778,7 +5776,7 @@ proto_registrar_dump_fields(int format) hfinfo->type == FT_INT64) { - switch(hfinfo->display & BASE_STRUCTURE_RESET) { + switch(hfinfo->display & BASE_DISPLAY_E_MASK) { case BASE_NONE: base_name = "BASE_NONE"; break; @@ -5850,8 +5848,8 @@ hfinfo_numeric_format(header_field_info *hfinfo) */ format = "%s == %u"; } else { - /* Pick the proper format string, ignoring BASE_RANGE_STRING flag */ - switch(hfinfo->display & ~BASE_RANGE_STRING) { + /* Get the underlying BASE_ value */ + switch(hfinfo->display & BASE_DISPLAY_E_MASK) { case BASE_DEC: case BASE_DEC_HEX: case BASE_OCT: /* I'm lazy */ @@ -5937,7 +5935,7 @@ construct_match_selected_string(field_info *finfo, epan_dissect_t *edt, DISSECTOR_ASSERT(hfinfo); abbrev_len = (int) strlen(hfinfo->abbrev); - if (hfinfo->strings && (hfinfo->display & BASE_STRUCTURE_RESET) == BASE_NONE) { + if (hfinfo->strings && (hfinfo->display & BASE_DISPLAY_E_MASK) == BASE_NONE) { const gchar *str = NULL; switch(hfinfo->type) { diff --git a/epan/proto.h b/epan/proto.h index 083953b40f..c0e146157d 100644 --- a/epan/proto.h +++ b/epan/proto.h @@ -140,15 +140,15 @@ typedef struct _protocol protocol_t; ep_strdup_printf("%s:%u: failed assertion \"%s\"", \ file, lineno, __DISSECTOR_ASSERT_STRINGIFY(expression)))) -/* BASE_STRUCTURE_RESET constant is used in proto.c to reset the bits - * identifying special structures used in translation of value for display. - * Its value means that we may have at most 16 base_display_e values */ -#define BASE_STRUCTURE_RESET 0x0F -/* Following constants have to be ORed with a base_display_e when dissector - * want to use specials MACROs (for the moment, only RVALS) for a - * header_field_info */ -#define BASE_RANGE_STRING 0x10 -/** radix for decimal values, used in header_field_info.display */ +/* Values for header_field_info.display */ + +/* For integral types, the display format is a base_display_e value + * possibly ORed with BASE_RANGE_STRING. */ + +/* BASE_DISPLAY_E_MASK selects the base_display_e value. Its current + * value means that we may have at most 16 base_display_e values. */ +#define BASE_DISPLAY_E_MASK 0x0F + typedef enum { BASE_NONE, /**< none */ BASE_DEC, /**< decimal */ @@ -159,14 +159,20 @@ typedef enum { BASE_CUSTOM /**< call custom routine (in ->strings) to format */ } base_display_e; +/* Following constants have to be ORed with a base_display_e when dissector + * want to use specials MACROs (for the moment, only RVALS) for a + * header_field_info */ +#define BASE_RANGE_STRING 0x10 + +/* BASE_ values that cause the field value to be displayed twice */ +#define IS_BASE_DUAL(b) ((b)==BASE_DEC_HEX||(b)==BASE_HEX_DEC) + typedef enum { HF_REF_TYPE_NONE, /**< Field is not referenced */ HF_REF_TYPE_INDIRECT, /**< Field is indirectly referenced (only applicable for FT_PROTOCOL) via. its child */ HF_REF_TYPE_DIRECT /**< Field is directly referenced */ } hf_ref_type; -#define IS_BASE_DUAL(b) ((b)==BASE_DEC_HEX||(b)==BASE_HEX_DEC) - /** information describing a header field */ typedef struct _header_field_info header_field_info; @@ -237,7 +243,9 @@ typedef struct field_info { /** The protocol field should not be shown in the tree (it's used for filtering only), * used in field_info.flags. */ -/* HIDING PROTOCOL FIELDS IS DEPRECATED, IT'S CONSIDERED TO BE BAD GUI DESIGN! */ +/* HIDING PROTOCOL FIELDS IS DEPRECATED, IT'S CONSIDERED TO BE BAD GUI DESIGN! + A user cannot tell by looking at the packet detail that the field exists + and that they can filter on its value. */ #define FI_HIDDEN 0x00000001 /** The protocol field should be displayed as "generated by Wireshark", * used in field_info.flags. */ diff --git a/gtk/dfilter_expr_dlg.c b/gtk/dfilter_expr_dlg.c index ae030a2f79..6b6c970e01 100644 --- a/gtk/dfilter_expr_dlg.c +++ b/gtk/dfilter_expr_dlg.c @@ -558,7 +558,7 @@ value_list_sel_cb(GtkTreeSelection *sel, gpointer value_entry_arg) * selected item, and display it in the base for this * field. */ - switch ((hfinfo->display) & BASE_STRUCTURE_RESET) { + switch ((hfinfo->display) & BASE_DISPLAY_E_MASK) { case BASE_NONE: case BASE_DEC: