Wireshark-bugs: [Wireshark-bugs] [Bug 8431] New Dissector Patch - USB PTP / MTP
Date: Mon, 19 Aug 2013 00:10:44 +0000

Comment # 43 on bug 8431 from
(In reply to comment #42)
> Hi Evan,
> 
> I'm able to pick this up again, sorry for the absence. 

No worries, we're always open :)

> (In reply to comment #40)
> > I was referring not just to the structure but to the accessor functions
> > (table_value_from_mask etc.) as well. That function references
> > USB_PTP_FLAVOR_MTP directly which is why I wasn't sure if it was possible to
> > make it generic or not.
> >
> > This does sound like something that could be made generic, although I'm
> > still not clear on the two-pass logic. From this explanation it sounds like
> > there should still only be one pass, but that entries not matching the mask
> > should be ignored rather than checked?
> > 
> 
> This is a PTP/MTP-specific behavior, so I guess the function is not a good
> candidate for being generic.  The vendor codes have collisions in their
> assignment.  On the first pass the code is looking for the more-specific,
> more-accurate vendor codes if they exist.  On the second pass the more
> generic MTP standard code is looked for.   Both codes exist in the same
> table w/ different masks and are matched at runtime.

Oh, OK, I think I finally understand. So the standard defines a code as X, but
Canon uses it for Y and Nikon uses it for Z. If we know the conversation is
(for example) with a Canon camera we check with the Canon mask, and if that
doesn't work, with the generic mask. If the conversation is with a Nikon
camera, we check with the Nikon mask, and then if necessary with the generic
mask.

Would it be simpler to split the table into several vendor-specific tables (one
for Canon, one for Nikon, etc) and one generic one? Then instead of the
complicated masking you can just do a regular lookup in the correct
vendor-specific table, and if that fails do a regular lookup in the generic
table.

> I've merged with most recent and everything is still compiling cleanly
> (modulo unrelated packet-dtn issues).  What's the next step in order to get
> this committed?     Thanks, -m

Depending on how much time/effort you have to spend on this it would be nice if
you could share whatever code or tables possible with the ptpip dissector that
is already in trunk. That's not a hard requirement, though I think it will be
pretty easy (many of the large mask tables you define are already in
packet-ptpip.h in one form or another).

There are also a few review comments left unaddressed from my comment #38 (and
the cppcheck warning in comment #40).

Other than that it's pretty close.


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