Hi,

I've looked at your code and looks close being ready for inclusion.
Here some steps to take:
- Convert to build in dissectors, there's no reason not to.
- Remove calls to check_col()
- In the header fields change the "" blubs by NULL
- You could look into the expert info system for error reporting.
- The occasional // style comment has to go
- some proto_tree_add_text() calls could be proto_tree_add_item() calls
- last parameter of proto_tree_add_item() is a gboolean, so either TRUE of FALSE
- Fuzztest them.
- And finally, add them to an enhancement bug.
- Protocol pages on the Wiki would be welcome documentation as well.

Thanks,
Jaap

Tobias Erichsen wrote:
> Hi Jaap,
> 
> like I wrote in my earlier mail, I will be converting the AppleMIDI
> dissector to be builtin in the next couple of days.
> 
> As the RTP-MIDI plugin is still work in progress (works perfectly
> already for standard-MIDI-data, but like I wrote does not fully
> decode more esoteric MIDI-message in all depths), I would prefer
> to keep that as plugin for the next time - would that be ok with
> the project?
> 
> I will also fuzztest the plugins, have not done that yet.
> 
> It would still be good if some of you guys who have done more work
> for Wireshark might take a small look at the code if that is in
> good shape for inclusion otherwise.
> 
> Best regards,
> Tobias
> 
>> -----Ursprüngliche Nachricht-----
>> Von: [email protected] 
>> [mailto:[email protected]] Im Auftrag von 
>> Jaap Keuter
>> Gesendet: Sonntag, 31. Januar 2010 22:41
>> An: Developer support list for Wireshark
>> Betreff: Re: [Wireshark-dev] RTP-MIDI and AppleMIDI dissectors
>>
>> Hi,
>>
>> The way to do this is to file an enhancement bug, so it won't 
>> get lost.
>>
>> Two questions up front:
>> Did you fuzztest the code?
>> Can you work both as build in dissectors? That is really the 
>> prefered way.
>>
>> Thanx,
>> Jaap
>>
>> Send from my iPhone
>>
>> On 31 jan 2010, at 14:19, "Tobias Erichsen" <[email protected]> wrote:
>>
>>> Hi everyone,
>>>
>>> after my initial efforts in 2006, I have since then reworked my 
>>> Previous code and have been able to compile it with the source of 
>>> Wireshark 1.2.6 both for x86 and x64.
>>>
>>> RTP-MIDI:
>>> Dissector-plugin to decode MIDI-data which is transmitted via RTP 
>>> based on RFC-4695.  The dissector currently supports the standard- 
>>> MIDI-stuff, but enhancements for more "esoteric" MIDI-sub-standards 
>>> still could to be implemented...
>>>
>>> AppleMIDI:
>>> Dissector-plugin to decode the lightweight 
>> Apple-network-MIDI session 
>>> establishment protocol (which is used to establish an 
>> RTP-session to 
>>> send & receive MIDI-data via RFC-4695).  This protocol is 
>> proprietary.  
>>> The dissector is based on the Apple implementation summary from May 
>>> 2005.
>>>
>>> You will find attached two zip-files containing the source-code and 
>>> one zip-file containing a sample-capture with both the 
>>> Apple-session-protocol and RF-4695-data.  This capture was done 
>>> between a MacBook and a MIDI Kiss-Box
>>>
>>> I would be glad if some of you guys could take a look at 
>> the code and 
>>> give me some feedback if this is already in good shape to be 
>>> integrated into the official Wireshark-release.
>>>
>>> I would be willing to rework the AppleMIDI plugin to work 
>> as a builtin 
>>> dissector, as this part is fully done.
>>>
>>> The RTP-MIDI-dissector should stay a plugin for a while 
>> since I plan 
>>> to implement more enhancements over the next couple of months 
>>> (decoding of some more specialized MIDI-commands for stuff like 
>>> lighting-control etc.)
>>>
>>> Best regards,
>>> Tobias
>>> <RTP-MIDI_plugin.zip>
>>> <AppleMIDI_plugin.zip>
>>> <RTP-MIDI_AppleMIDI_data.zip>
>>>

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to