From Alexey Neyman :

This patch implements a function for dissecting bitfields with better control
over the resulting representation than the existing proto_tree_add_bitmask()
routine. This function will be used by reworked IPMI/ATCA dissector (bug 2048).

The function is described in README.developer. In short, the differences are as
follows:

- The new function does not require a hf_XXX field for the whole bitmask. When
the bitmask includes several unrelated fields, such hf_XXX field does not make
sense.

- The new function allows better control over the way the sub-item descriptions
are added to the top-level item. For example, proto_tree_add_bitmask() function
does not add non-enumerated integers, does not use true_false_string to display
boolean.

- The new function allows to specify "fallback" text for the top-level item
which is used if no items were added to the top-level item.

svn path=/trunk/; revision=25920

doc/README.developer

@@ -2018,6 +2018,11 @@ protocol or field labels to the proto_tree:
 	proto_tree_add_bitmask(tree, tvb, start, header, ett, **fields,
 	    little_endian);
 
+	proto_item *
+	proto_tree_add_bitmask_text(proto_tree *tree, tvbuff_t *tvb,
+	    guint offset, guint len, const char *name, const char *fallback,
+            gint ett, const int **fields, gboolean little_endian, int flags);
+
 The 'tree' argument is the tree to which the item is to be added.  The
 'tvb' argument is the tvbuff from which the item's value is being
 extracted; the 'start' argument is the offset from the beginning of that
@@ -2318,8 +2323,8 @@ This is like proto_tree_add_text(), but takes, as the last argument, a
 variable-length list of arguments to add a text item to the protocol
 tree.
 
-proto_tree_add_bitmask()
+proto_tree_add_bitmask() and proto_tree_add_bitmask_text()
-------------------------
+----------------------------------------------------------
 This function provides an easy to use and convenient helper function
 to manage many types of common bitmasks that occur in protocols.
 
@@ -2430,6 +2435,24 @@ filter is then possible:
 
 	tr.rif_ring eq 0x013
 
+The proto_tree_add_bitmask_text() function is an extended version of
+the proto_tree_add_bitmask() function. In addition, it allows to:
+- Provide a leading text (e.g. "Flags: ") that will appear before
+  the comma-separated list of field values
+- Provide a fallback text (e.g. "None") that will be appended if
+  no fields warranted a change to the top-level title.
+- Using flags, specify which fields will affect the top-level title.
+
+There are the following flags defined:
+
+  BMT_NO_APPEND - the title is taken "as-is" from the 'name' argument.
+  BMT_NO_INT - only boolean flags are added to the title.
+  BMT_NO_FALSE - boolean flags are only added to the title if they are set.
+  BMT_NO_TFS - only add flag name to the title, do not use true_false_string
+
+The proto_tree_add_bitmask() behavior can be obtained by providing
+both 'name' and 'fallback' arguments as NULL, and a flags of
+(BMT_NO_FALSE|BMT_NO_TFS).
 
 1.7 Utility routines.
 

epan/proto.c

@@ -202,6 +202,9 @@ static void
 proto_tree_set_uint64(field_info *fi, guint64 value);
 static void
 proto_tree_set_uint64_tvb(field_info *fi, tvbuff_t *tvb, gint start, guint length, gboolean little_endian);
+static gboolean
+proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, int offset, int len, gint ett,
+	const gint **fields, gboolean little_endian, int flags, gboolean first);
 
 static int proto_register_field_init(header_field_info *hfinfo, int parent);
 
@@ -5695,6 +5698,124 @@ proto_construct_match_selected_string(field_info *finfo, epan_dissect_t *edt)
 }
 
 
+/* This function is common code for both proto_tree_add_bitmask() and
+ *  proto_tree_add_bitmask_text() functions.
+ */
+static gboolean
+proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, int offset, int len, gint ett,
+	const int **fields, gboolean little_endian, int flags, gboolean first)
+{
+	guint32 value = 0, tmpval;
+	proto_tree *tree = NULL;
+	header_field_info *hf;
+	const char *fmt;
+
+	switch (len) {
+	case 1:
+		value = tvb_get_guint8(tvb, offset);
+		break;
+	case 2:
+		value = little_endian ? tvb_get_letohs(tvb, offset) :
+			tvb_get_ntohs(tvb, offset);
+		break;
+	case 3:
+		value = little_endian ? tvb_get_letoh24(tvb, offset) :
+			tvb_get_ntoh24(tvb, offset);
+		break;
+	case 4:
+		value = little_endian ? tvb_get_letohl(tvb, offset) :
+			tvb_get_ntohl(tvb, offset);
+		break;
+	default:
+		g_assert_not_reached();
+	}
+
+	tree = proto_item_add_subtree(item, ett);
+	while (*fields) {
+		proto_tree_add_item(tree, **fields, tvb, offset, len, little_endian);
+		if (flags & BMT_NO_APPEND) {
+			fields++;
+			continue;
+		}
+		hf = proto_registrar_get_nth(**fields);
+		DISSECTOR_ASSERT(hf->bitmask != 0);
+		tmpval = (value & hf->bitmask) >> hf->bitshift;
+
+		switch (hf->type) {
+		case FT_INT8:
+		case FT_UINT8:
+		case FT_INT16:
+		case FT_UINT16:
+		case FT_INT24:
+		case FT_UINT24:
+		case FT_INT32:
+		case FT_UINT32:
+			DISSECTOR_ASSERT(len == ftype_length(hf->type));
+
+			if (hf->display == BASE_CUSTOM) {
+				gchar lbl[ITEM_LABEL_LENGTH];
+				custom_fmt_func_t fmtfunc = (custom_fmt_func_t)hf->strings;
+
+				DISSECTOR_ASSERT(fmtfunc);
+				fmtfunc(lbl, tmpval);
+				proto_item_append_text(item, "%s%s: %s", first ? "" : ", ",
+						hf->name, lbl);
+				first = FALSE;
+			}
+			else if (hf->strings) {
+				proto_item_append_text(item, "%s%s: %s", first ? "" : ", ",
+						hf->name, val_to_str(tmpval, hf->strings, "Unknown"));
+				first = FALSE;
+			}
+			else if (!(flags & BMT_NO_INT)) {
+				if (!first) {
+					proto_item_append_text(item, ", ");
+				}
+
+				fmt = IS_FT_INT(hf->type) ? hfinfo_int_format(hf) : hfinfo_uint_format(hf);
+				if (IS_BASE_DUAL(hf->display)) {
+					proto_item_append_text(item, fmt, hf->name, tmpval, tmpval);
+				} else {
+					proto_item_append_text(item, fmt, hf->name, tmpval);
+				}
+				first = FALSE;
+			}
+
+			break;
+		case FT_BOOLEAN:
+			DISSECTOR_ASSERT(len * 8 == hf->display);
+
+			if (hf->strings && !(flags & BMT_NO_TFS)) {
+				/* If we have true/false strings, emit full - otherwise messages
+				   might look weird */
+				const struct true_false_string *tfs =
+					(const struct true_false_string *)hf->strings;
+
+				if (tmpval) {
+					proto_item_append_text(item, "%s%s: %s", first ? "" : ", ",
+							hf->name, tfs->true_string);
+					first = FALSE;
+				} else if (!(flags & BMT_NO_FALSE)) {
+					proto_item_append_text(item, "%s%s: %s", first ? "" : ", ",
+							hf->name, tfs->false_string);
+					first = FALSE;
+				}
+			} else if (hf->bitmask & value) {
+				/* If the flag is set, show the name */
+				proto_item_append_text(item, "%s%s", first ? "" : ", ", hf->name);
+				first = FALSE;
+			}
+			break;
+		default:
+			g_assert_not_reached();
+		}
+
+		fields++;
+	}
+
+	return first;
+}
+
 /* This function will dissect a sequence of bytes that describe a
  * bitmask.
  * hf_hdr is a 8/16/24/32 bit integer that describes the bitmask to be dissected.
@@ -5708,105 +5829,45 @@ proto_construct_match_selected_string(field_info *finfo, epan_dissect_t *edt)
  * This array is terminated by a NULL entry.
  *
  * FT_BOOLEAN bits that are set to 1 will have the name added to the expansion.
- * FT_integer fields that have a value_string attached will have the
+ * FT_integer fields that have a value_string attached will have the 
  * matched string displayed on the expansion line.
  */
 proto_item *
-proto_tree_add_bitmask(proto_tree *parent_tree, tvbuff_t *tvb, int offset, int hf_hdr, gint ett, const int **fields, gboolean little_endian)
+proto_tree_add_bitmask(proto_tree *parent_tree, tvbuff_t *tvb, guint offset, int hf_hdr,
+		gint ett, const int **fields, gboolean little_endian)
 {
-	proto_tree *tree=NULL;
+	proto_item *item = NULL;
-	proto_item *item=NULL;
+	header_field_info *hf;
-	header_field_info *hf_info;
+	int len;
-	int len=0;
-	guint32 value=0;
 
-	hf_info=proto_registrar_get_nth(hf_hdr);
+	hf = proto_registrar_get_nth(hf_hdr);
-	switch(hf_info->type){
+	DISSECTOR_ASSERT(IS_FT_INT(hf->type) || IS_FT_UINT(hf->type));
-	case FT_INT8:
+	len = ftype_length(hf->type);
-	case FT_UINT8:
+
-		len=1;
+	if (parent_tree) {
-		value=tvb_get_guint8(tvb, offset);
+		item = proto_tree_add_item(parent_tree, hf_hdr, tvb, offset, len, little_endian);
-		break;
+		proto_item_add_bitmask_tree(item, tvb, offset, len, ett, fields, little_endian,
-	case FT_INT16:
+				BMT_NO_INT|BMT_NO_TFS, FALSE);
-	case FT_UINT16:
-		len=2;
-		if(little_endian){
-			value=tvb_get_letohs(tvb, offset);
-		} else {
-			value=tvb_get_ntohs(tvb, offset);
-		}
-		break;
-	case FT_INT24:
-	case FT_UINT24:
-		len=3;
-		if(little_endian){
-			value=tvb_get_letoh24(tvb, offset);
-		} else {
-			value=tvb_get_ntoh24(tvb, offset);
-		}
-		break;
-	case FT_INT32:
-	case FT_UINT32:
-		len=4;
-		if(little_endian){
-			value=tvb_get_letohl(tvb, offset);
-		} else {
-			value=tvb_get_ntohl(tvb, offset);
-		}
-		break;
-	default:
-		g_assert_not_reached();
 	}
 
-	if(parent_tree){
+	return item;
-		item=proto_tree_add_item(parent_tree, hf_hdr, tvb, offset, len, little_endian);
+}
-		tree=proto_item_add_subtree(item, ett);
-	}
 
-	while(*fields){
+/* The same as proto_tree_add_bitmask(), but using an arbitrary text as a top-level item */
-		header_field_info *hf_field;
+proto_item *
-		guint32 tmpval, tmpmask;
+proto_tree_add_bitmask_text(proto_tree *parent_tree, tvbuff_t *tvb, guint offset, guint len,
+		const char *name, const char *fallback,
+		gint ett, const int **fields, gboolean little_endian, int flags)
+{
+	proto_item *item = NULL;
 
-		hf_field=proto_registrar_get_nth(**fields);
+	if (parent_tree) {
-		switch(hf_field->type){
+		item = proto_tree_add_text(parent_tree, tvb, offset, len, "%s", name ? name : "");
-		case FT_INT8:
+		if (proto_item_add_bitmask_tree(item, tvb, offset, len, ett, fields, little_endian,
-		case FT_UINT8:
+					flags, TRUE) && fallback) {
-		case FT_INT16:
+			/* Still at first item - append 'fallback' text if any */
-		case FT_UINT16:
+			proto_item_append_text(item, "%s", fallback);
-		case FT_INT24:
-		case FT_UINT24:
-		case FT_INT32:
-		case FT_UINT32:
-			proto_tree_add_item(tree, **fields, tvb, offset, len, little_endian);
-
-			/* Mask and shift out the value */
-			tmpmask=hf_field->bitmask;
-			tmpval=value;
-			if(tmpmask){
-				tmpval&=tmpmask;
-				while(!(tmpmask&0x00000001)){
-					tmpval>>=1;
-					tmpmask>>=1;
-				}
-			}
-			/* Show the value_string content (if there is one) */
-			if(hf_field->strings && hf_field->display != BASE_CUSTOM){
-				proto_item_append_text(item, ",  %s", val_to_str(tmpval, hf_field->strings, "Unknown"));
-			}
-
-			break;
-		case FT_BOOLEAN:
-			proto_tree_add_item(tree, **fields, tvb, offset, len, little_endian);
-			/* if the flag is set, show the name */
-			if(hf_field->bitmask&value){
-				proto_item_append_text(item, ",  %s", hf_field->name);
-			}
-			break;
-		default:
-			g_assert_not_reached();
 		}
-
-		fields++;
 	}
 
 	return item;

epan/proto.h

@@ -1539,6 +1539,7 @@ proto_find_field_from_offset(proto_tree *tree, guint offset, tvbuff_t *tvb);
         This field will form an expansion under which the individual fields of the
         bitmask is dissected and displayed.
         This field must be of the type FT_[U]INT{8|16|24|32}.
+ @param ett subtree index
  @param fields an array of pointers to int that lists all the fields of the
         bitmask. These fields can be either of the type FT_BOOLEAN for flags
         or another integer of the same type/size as hf_hdr with a mask specified.
@@ -1549,7 +1550,28 @@ proto_find_field_from_offset(proto_tree *tree, guint offset, tvbuff_t *tvb);
  @param little_endian big or little endian byte representation
  @return the newly created item */
 extern proto_item *
-proto_tree_add_bitmask(proto_tree *tree, tvbuff_t *tvb, int offset, int hf_hdr, gint ett, const int **fields, gboolean little_endian);
+proto_tree_add_bitmask(proto_tree *tree, tvbuff_t *tvb, guint offset,
+		int hf_hdr, gint ett, const int **fields, gboolean little_endian);
+
+/** Add a text with a subtree of bitfields.
+ @param tree the tree to append this item to
+ @param tvb the tv buffer of the current data
+ @param offset start of data in tvb
+ @param name field name (NULL if bitfield contents should be used)
+ @param fallback field name if none of bitfields were usable
+ @param ett subtree index
+ @param fields NULL-terminated array of bitfield indexes
+ @param little_endian big or little endian byte representation
+ @return the newly created item */
+extern proto_item *
+proto_tree_add_bitmask_text(proto_tree *tree, tvbuff_t *tvb, guint offset, guint len,
+		const char *name, const char *fallback,
+		gint ett, const int **fields, gboolean little_endian, int flags);
+
+#define BMT_NO_APPEND	0x01	/**< Don't change the title at all */
+#define BMT_NO_INT	0x02	/**< Don't add integral (non-boolean) fields to title */
+#define BMT_NO_FALSE	0x04	/**< Don't add booleans unless they're TRUE */
+#define BMT_NO_TFS	0x08	/**< Don't use true_false_string while formatting booleans */
 
 /** Add bits to a proto_tree, using the text label registered to that item.
    The item is extracted from the tvbuff handed to it.