Wireshark · Wireshark-dev: [Wireshark-dev] Question on format_value variant of proto_tree_add_bits_item funtion

[Gerasimos Dimitriadis <gedimitr@xxxxxxxxx>]

Hi all,

I've been looking for a _format_value variant of the proto_tree_add_bits_item
function, because of the increased flexibility.

My initial approach was to create a single function that can deal with
all (at least the most common)
types of header fields, so I was thinking of passing the reference of
the field value
as a void * and then casting back to the correct type according to
hf_field->type. This means
that the user *must* provide a pointer to a value as expected in the
definition of the corresponding proto_tree_add_xxx_format.
So essentially I was thinking about something like the one below:

	...
	case FT_UINT8:
	case FT_UINT16:
	case FT_UINT24:
	case FT_UINT32:
		/* 1 - 32 unsigned bits field */
		return proto_tree_add_uint_format(tree, hf_index, tvb, offset,
length, *(guint32 *)value_ptr,
				"%s: %s", str, value_str);
		break;
		
	case FT_FLOAT:
		return proto_tree_add_float_format(tree, hf_index, tvb, offset,
length, *(float *)value_ptr,
				"%s: %s", str, value_str);
		break;
	...
	
On the one  hand, this is a very compact way of handling this problem,
but on the other hand
this can potentially lead to subtle, hard to find bugs, since the
compiler's type
checking is no longer active, and integers of different sizes cannot
be used interchangeably.

Alternatively, bits variants of the proto_tree_add_xxx_format can be
created, which is a much safer course.

What is your opinion on this issue?

Best regards,

Gerasimos