Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
Le 27 août 2014 02:25, Stalley, Sean sean.stal...@intel.com a écrit : Martin, I was able to set up a notification via the web interface. Under Username-Settings-Watched Projects Set project to: wireshark and the conditional to: file:epan/dissectors/packet-usb.c I think we should discuss how to clean up the usb dissector. It seems like we are running into the too many cooks in the kitchen scenario. My primary goal has been to reuse as much of the USB dissector as possible for the MA USB dissector. I've been trying to simplify/modularize dissect_usb_common() along the way, but if I'm just getting in the way let me know. Pascal, Sorry for breaking the regression. Are there any BKMs for setting up regression testing (other than a script that diffs tshark outputs)? Hi Sean, I'm using a basic diff of tshark output. Pascal. Thanks, Sean ___ 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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
Hi Sean, Thus wrote Stalley, Sean (sean.stal...@intel.com): I think we should discuss how to clean up the usb dissector. It seems like we are running into the too many cooks in the kitchen scenario. So far, our changes co-existed nicely. Now's the first time they were in conflict ;-) My primary goal has been to reuse as much of the USB dissector as possible for the MA USB dissector. I've been trying to simplify/modularize dissect_usb_common() along the way, but if I'm just getting in the way let me know. I appreciate your work, the USB dissector definitely needs more cleanup. As I said in the previous mail, I'd like to reduce the number of state variables and simplify the execution flow (there's too many conditions and branches). I'll upload some more changes tomorrow. Best regards, Martin ___ 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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
On 27 August 2014 23:26, Martin Kaiser li...@kaiser.cx wrote: I appreciate your work, the USB dissector definitely needs more cleanup. Not only USB. Also next dissectors like HID, masstorage, etc. which does not work with DecodeAs. -- Pozdrawiam / Best regards - Michał Łabędzki, Software Engineer Tieto Corporation Product Development Services http://www.tieto.com / http://www.tieto.pl --- ASCII: Michal Labedzki location: Swobodna 1 Street, 50-088 Wrocław, Poland room: 5.01 (desk next to 5.08) --- Please note: The information contained in this message may be legally privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any unauthorised use, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to the message and deleting it from your computer. Thank You. --- Please consider the environment before printing this e-mail. --- Tieto Poland spółka z ograniczoną odpowiedzialnością z siedzibą w Szczecinie, ul. Malczewskiego 26. Zarejestrowana w Sądzie Rejonowym Szczecin-Centrum w Szczecinie, XIII Wydział Gospodarczy Krajowego Rejestru Sądowego pod numerem 124858. NIP: 8542085557. REGON: 812023656. Kapitał zakładowy: 4 271500 PLN ___ 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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
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.
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-re...@wireshark.org):
URL:
https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=64cd8270c669e35bc2911133a9a7da31c4bb1965
Submitter: Anders Broman (a.broma...@gmail.com)
Changed: branch: master
Repository: wireshark
Commits:
64cd827 by Sean O. Stalley (sean.stal...@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.broma...@gmail.com
Reviewed-by: Anders Broman a.broma...@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-comm...@wireshark.org
Archives:http://www.wireshark.org/lists/wireshark-commits
Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
mailto:wireshark-commits-requ...@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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
Hi Martin,
2014-08-26 12:05 GMT+02:00 Martin Kaiser li...@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-re...@wireshark.org):
URL:
https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=64cd8270c669e35bc2911133a9a7da31c4bb1965
Submitter: Anders Broman (a.broma...@gmail.com)
Changed: branch: master
Repository: wireshark
Commits:
64cd827 by Sean O. Stalley (sean.stal...@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.broma...@gmail.com
Reviewed-by: Anders Broman a.broma...@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-comm...@wireshark.org
Archives:http://www.wireshark.org/lists/wireshark-commits
Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
mailto:wireshark-commits-requ...@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-requ...@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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
On 26 August 2014 11:05, Martin Kaiser li...@kaiser.cx wrote: Is there a way to subscribe to gerrit change requests so that I'm notified when a new USB-related change is uploaded? I have no idea if this will do what you want, but there does appear to be email notification from Gerrit: https://code.wireshark.org/review/Documentation/user-notify.html -- Graham Bloice ___ 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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
Hi Pascal, Thus wrote Pascal Quantin (pascal.quan...@gmail.com): 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? looks good to me, I just merged it. Best regards, Martin ___ 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-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master 64cd827: Have dissect_usb_setup_request handle non-standard requests
Martin, I was able to set up a notification via the web interface. Under Username-Settings-Watched Projects Set project to: wireshark and the conditional to: file:epan/dissectors/packet-usb.c I think we should discuss how to clean up the usb dissector. It seems like we are running into the too many cooks in the kitchen scenario. My primary goal has been to reuse as much of the USB dissector as possible for the MA USB dissector. I've been trying to simplify/modularize dissect_usb_common() along the way, but if I'm just getting in the way let me know. Pascal, Sorry for breaking the regression. Are there any BKMs for setting up regression testing (other than a script that diffs tshark outputs)? Thanks, Sean ___ 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-requ...@wireshark.org?subject=unsubscribe
