https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5370
--- Comment #10 from Chris Maynard <christopher.maynard@xxxxxxxxx> 2010-11-23 09:03:19 PST ---
Some comments on the proposed patch:
pcap-common.c:
1) Shouldn't lines 1095 and 1132 read something like the following to account
for both transfers in or out?:
if ((phdr->transfer_type & ~URB_TRANSFER_IN) == URB_ISOCHRONOUS) {
2) The iso_numdesc isn't really needed since phdr->s.iso.numdesc can be
directly used in the for() loop at line 1140 instead.
packet-usb.c:
1) All multi-byte fields in the USB pseudo header have been byte-swapped, if
needed, in pcap-common.c so that the values are now in host byte order, i.e.
the host doing the dissecting. This means that none of these pseudo header
fields can be added using proto_tree_add_item(tree, hf_xyz, tvb, offset,
length, endian), since "endian" can only be ENC_BIG_ENDIAN or
ENC_LITTLE_ENDIAN. There is no ENC_HOST_ENDIAN. So ... where the ISO
hf_usb_iso_error_count and hf_usb_iso_numdesc fields are added at ~lines
2200-2208, this has to instead mimic what dissect_linux_usb_pseudo_header()
does by doing tvb_memcpy()'s and then calling the proto_tree_add_uint(). This
applies down around ~lines 2242-2254 as well with hf_usb_iso_status,
hf_usb_iso_off, hf_usb_iso_len, and even hf_usb_iso_pad.
2) I'm confused with this block of code:
if (!iso_status && iso_len && data_base + iso_off + iso_len <=
tvb_length(tvb))
proto_tree_add_item(tree, hf_usb_iso_data, tvb, data_base +
iso_off, iso_len, TRUE);
offset += 4;
You are adding some data in between the iso_len and iso_pad? Is the iso_pad
only present if there is no data? But if that's the case, then why always add
the iso_pad? Should the data come after the iso_pad, since the mon_bin_isodesc
struct is defined as follows?:
struct mon_bin_isodesc {
int iso_status;
unsigned int iso_off;
unsigned int iso_len;
u32 _pad;
};
3) For hf_usb_iso_off and hf_usb_iso_len, I think "Offset [bytes]" and "Length
[bytes]" can just be "Offset" and "Length". If necessary (which I don't think
it is), use the blurb field for a more detailed description of the field.
Marton, do you have a small sample packet capture file that you could attach
with USB isochronous packets?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
