Re: [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests

[Pascal Quantin <pascal.quantin () gmail com>]

Hi Martin,

2014-08-26 12:05 GMT+02:00 Martin Kaiser <lists () kaiser cx>:

> The new req_type is the same as the existing type. There's a
> switch(type) {...} followed by in if (req_type==...).
>
> My goal is to have only one call to try_dissect_next_protocol(). In
> dissect_usb_common(), we dissect the standard fields in the main
> switch-statement and call try_dissect_next_protocol() _once_ for the
> remaining data.
>
> At the moment, try_dissect_next_protocol() is called
> twice for control requests that it can't handle. This adds two generated
> items saying "unknown class".

I spotted a regression myself (that seems similar to what you report) and
proposed https://code.wireshark.org/review/#/c/3858/ to fix it. Could you
give it a try on your sample?



> Unfortunately, the recent patches make it harder to fix this...
>
> Why is the setup_tvb for the standard setup request generated in
> dissect_nonstandard_usb_setup_request()?
>
> if (header_info & (USB_HEADER_IS_LINUX | USB_HEADER_IS_64_BYTES)) {
> ...
> }
> else {
> ...
>         offset = try_dissect_linux_usb_pseudo_header_ext(tvb, offset,
> pinfo, tree, header_info);
>     }
>
> Will try_dissect_linux_usb_pseudo_header_ext() ever do anything if it's
> only called when there's no extended pseudo header?
>
> Is there a way to "subscribe" to gerrit change requests so that I'm
> notified when a new USB-related change is uploaded?
>
> Thus wrote Wireshark code review (code-review-do-not-reply () wireshark org):
>
> > URL:
> https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=64cd8270c669e35bc2911133a9a7da31c4bb1965
> > Submitter: Anders Broman (a.broman58 () gmail com)
> > Changed: branch: master
> > Repository: wireshark
>
> > Commits:
>
> > 64cd827 by Sean O. Stalley (sean.stalley () intel com):
>
> >     Have dissect_usb_setup_request handle non-standard requests
>
> >     Moved code for parsing non-standard setup requests from
> >     dissect_usb_common() to dissect_usb_setup_request().
>
> >     Also added header_info flag USB_HEADER_IS_MAUSB & updated mausb
> >     dissector.
>
> >     Change-Id: Ifa8abccbd57bf4dd3965f582872952383e6f737d
> >     Reviewed-on: https://code.wireshark.org/review/3851
> >     Petri-Dish: Anders Broman <a.broman58 () gmail com>
> >     Reviewed-by: Anders Broman <a.broman58 () gmail com>
>
>
> > Actions performed:
>
> >     from  3d4d021   Non-standard USB control requests now handled in own
> function
> >     adds  64cd827   Have dissect_usb_setup_request handle non-standard
> requests
>
>
> > Summary of changes:
> >  epan/dissectors/packet-mausb.c |    9 +++----
> >  epan/dissectors/packet-usb.c   |   51
> ++++++++++++++++++++++------------------
> >  epan/dissectors/packet-usb.h   |    7 ++++--
> >  3 files changed, 38 insertions(+), 29 deletions(-)
> ___________________________________________________________________________
> > Sent via:    Wireshark-commits mailing list <
> wireshark-commits () wireshark org>
> > Archives:    http://www.wireshark.org/lists/wireshark-commits
> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
> >              mailto:wireshark-commits-request () wireshark org
> ?subject=unsubscribe
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request () wireshark org
> ?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe