Wireshark-dev: Re: [Wireshark-dev] RTP-MIDI and AppleMIDI dissectors

From: "Tobias Erichsen" <t.erichsen@xxxxxx>
Date: Wed, 3 Feb 2010 06:43:58 +0100
Hi Jaap,

thanks for your feedback:

> - Convert to build in dissectors, there's no reason not to.

Done that.

> - Remove calls to check_col()

Why is that?  I took the template-code for dissectors from the
README.developer
where it explicitely states that one should check if this column is active -
is this not valid anymore?

> - In the header fields change the "" blubs by NULL

Done that.

> - You could look into the expert info system for error reporting.
> - The occasional // style comment has to go

Shoot - overlooked the one C++ comment in RTP-MIDI ;-)

> - some proto_tree_add_text() calls could be proto_tree_add_item() calls

Actually I have tried to move as many of the ...add_text calls to
...add_item calls.
In cases where I have used the ...add_text, not enough information was
available,
or I had to take information from several places to get some more
informative
text displayed which I could not automatically generate with the items...

If you have any specific place in mind, which I should change,  I would be
glad if
you could give me a pointer to a line in my code.

> - last parameter of proto_tree_add_item() is a gboolean, so  either TRUE
of FALSE

Done that.

> - Fuzztest them.

Done that. No problems found.

> - And finally, add them to an enhancement bug.

I will be doing this as soon as the remaining questions/topics above have
been
Resolved...

> - Protocol pages on the Wiki would be welcome documentation as well.

I will look into that.

Best regards,
Tobias