Wireshark-bugs: [Wireshark-bugs] [Bug 8431] New Dissector Patch - USB PTP / MTP

Date: Thu, 18 Apr 2013 05:04:34 +0000

Comment # 39 on bug 8431 from
(In reply to comment #38)
> Regarding masked value strings, I'm still not clear on the intention here.
> This is likely due to the fact that I don't have any real understanding of
> the protocol, but the two-pass check in table_value_from_mask is very
> confusing.
> 
> If this is something that might be generically useful then the entire thing
> (including table_value_from_mask and related) should be split off and made
> generic (ie not hard-coding USB_PTP_FLAVOR_MTP). However, given the odd
> nature of I am no longer sure - again I don't really understand what it's
> supposed to be doing.
> 

I already made it generic in the above patch.   The idea is that each item in
the table may or may not be valid depending on the state of the conversation.  
This makes the tables a dynamic lookup instead of a static one.   Example :
connection handles MTP or not, so we do or don't use entries that have the MTP
flag.   More than one flag can be set at a time. 

>  - Don't name a function proto_tree_* unless it's part of the API, just to
> avoid confusion. Unless it should be in epan/proto.c (which it might,
> depending on what we end up doing with the masked value strings?) then call
> it something else; usb_ptp_add_mask is probably fine.

OK, i'll rename it

>  - For all boolean parameters or variables, please use the type 'gboolean'
> and the literals TRUE and FALSE.

Roger.

> - Your semi-auto-generated masked value string tables don't end in NULL
> markers, so unmatched values will run off the end of the array into bad
> memory.

Will fix.

> - On my machine the compiler spits out an enourmous number of errors, I
> think you're missing an include or something?

I can't reproduce this, I'm able to compile in Linux and Windows, 32bit and
64bit.   You'll have to give me more output here.


You are receiving this mail because:
  • You are watching all bug changes.