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.

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@31249 f5534014-38df-0310-8fa8-9805f1628bb7

diff --git a/epan/proto.c b/epan/proto.c
index af01ce2959b35ad0538a3a11b5d6f641aab80823..6256435af86a602ef97e1a89040150b3ef9e0d67 100644 (file)
--- 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 083953b40f9aebbd1f1ef4542d3097ee10f0945b..c0e146157dbec4ebe2ef349f8d7d6c35a38de30a 100644 (file)
--- 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 ae030a2f790ec39a989000458de164bfe84fe776..6b6c970e01d9b2ebd3cf189369b11a9ae8c99ca1 100644 (file)
--- 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: