On Mon, Mar 06, 2000 at 01:53:13PM -0800, Guy Harris wrote:
>
> Perhaps, instead of a single "_proto_tree_add_item_value()" routine,
> there should be multiple such routines, with the value being part of the
> *fixed* portion of the argument list. This would also let us do the
> same with "proto_tree_add_item()" and the like, which would not only let
> us do format checking on "proto_tree_add_item_format()", it would *also*
> let us catch some incorrect calls to the various "proto_tree_add_item"
> routines, e.g. passing a pointer if the actual item is of an integral
> type.
I see 3 different ways for adding the format to the fixed portion of
the argument list of proto_tree_add_item_format(), and they're all ugly.
<aside>
Of course, I never really like proto_tree_add_item_format, because
I feel that everything should be generic enough to use
proto_tree_add_item(). That's a lofty goal, but one we have to consider
when we start moving towards libdencode, where Ethereal will just take
the decode data from libdencode and displays it in the GUI.
</aside>
Idea #1
=======
Since the format and the args for that format usually go last in the
argument list, the value of the field has to go before the format. So,
we'll end up with lots of functions:
proto_tree_add_item_format_bytes(proto_tree* tree, int hfindex,
gint start, gint length, guint8* value, const char *format, ...);
proto_tree_add_item_format_boolean(proto_tree* tree, int hfindex,
gint start, gint length, unsigned int value, const char *format, ...);
proto_tree_add_item_format_uint8(proto_tree* tree, int hfindex,
gint start, gint length, guint8 value, const char *format, ...);
proto_tree_add_item_format_uint16(proto_tree* tree, int hfindex,
gint start, gint length, guint16 value, const char *format, ...);
proto_tree_add_item_format_uint24(proto_tree* tree, int hfindex,
gint start, gint length, guint32 value, const char *format, ...);
proto_tree_add_item_format_uint32(proto_tree* tree, int hfindex,
gint start, gint length, guint32 value, const char *format, ...);
etc...
Idea #2
=======
We *could* put in a constraint saying that the first argument after
the format *has* to be the field value. The first field after the format
would be used both for tucking the value away in the field_info struct, and
for printing in the GUI tree.
But there might be a case where the programmer does not want the
field's value to be the first thing the in format.
Idea #3
=======
The format and its arguments don't *have* to be contiguous in order
for GCC to check the syntax. The GCC __attribute__ modifier lets us
give both the index for the format and the index for the start of the args:
/* Set text of proto_item after having already been created. */
#if __GNUC__ == 2
void proto_item_set_text(proto_item *ti, const char *format, ...)
__attribute__((format (printf, 2, 3)));
#else
void proto_item_set_text(proto_item *ti, const char *format, ...);
#endif
We *could* put the field value as the first argument after the format,
and then have the format arguments come after that. That would result
in a non-standard way of doing printf-style format calls, but we'd
end up with a single function:
proto_tree_add_item_format(proto_tree* tree, int hfindex,
gint start, gint length, const char *format, ...)
__attribute__((format (printf, 5, 7)));
(See how arg 5 is the format, and arg 7 is the beginning of the list.
That means that arg 6 is free for us to use).
Since this API is so non-standard, it might cause grief to programmers
(but we'd find the problem at compile-time! :-)
Thoughts?
--gilbert