Wireshark-dev: Re: [Wireshark-dev] [Wireshark-bugs] [Bug 3138] New: Buildbot crash output: fuzz
From: "Martin Mathieson" <martin.r.mathieson@xxxxxxxxxxxxxx>
Date: Fri, 19 Dec 2008 10:46:34 +0000


On Thu, Dec 18, 2008 at 9:16 PM, Guy Harris <guy@xxxxxxxxxxxx> wrote:

On Dec 18, 2008, at 1:03 PM, Martin Mathieson wrote:

> Sorry about that, I didn't grep to see if it was being called.
> Because it compiled OK I wrongly assumed it wasn't.

BTW, as it's an add_bytes routine, presumably it should take a const
guint8 * pointing to the sequence of bytes to be added, the way
proto_tree_add_bytes() and proto_tree_add_bytes_format_value() do.
Why was the start_ptr argument removed?


I didn't look closely enough at the surrounding functions to see what the convention was.

There was no difference in the arg lists between proto_tree_add_bytes_format_value() and proto_tree_add_bytes_format().  The name suggested to me that the addition of 'value' would mean a separate value was being passed in, rather than taking the value from the tvb that it was highlighting.

I've since read the comments in proto.h more carefully, and it looks as though the difference is whether or not the hf items's label will include the fixed string before the formatted text or not.

I wanted to add a field that took its value from the a range of bytes in the tvb, but I wanted control over what the label looked like (and I didn't want to the the truncated sequence of bytes in the item).  I couldn't see anyway to do this, other than doing proto_tree_add_item() followed by proto_item_set_text().  Am I missing something obvious here?

Looking at Gerald's change 27053, there were lots of functions that wanted to do the same thing as I wanted, but they had to call tvb_get_ptr() just to get the data the match the selected range of bytes.

Once again, I apologise for my rash change - but I think there's the need for a function that does what I changed proto_tree_add_bytes_format() to do.

What should we do now?
Martin