Fixed
Status Update
Comments
el...@google.com <el...@google.com>
ap...@google.com <ap...@google.com> #2
Correction:
So, the end result is that usb_write() sometimes sends a ZLP when it doesn't need to
should be
So, the end result is that usb_write() sometimes sends a ZLP when it doesn't need to, and sometimes neglects to send a ZLP when it should
So, the end result is that usb_write() sometimes sends a ZLP when it doesn't need to
should be
So, the end result is that usb_write() sometimes sends a ZLP when it doesn't need to, and sometimes neglects to send a ZLP when it should
ap...@google.com <ap...@google.com> #3
Note that the CHECK_LE has since been replaced with HandleError(), which causes adbd to not abort...but that stowaway amessage header is being dropped all the same...and so the behavior is pretty much undefined, I imagine. I'm not sure the switch to HandleError was a good idea, since it more or less swept the real problem under the rug.
ap...@google.com <ap...@google.com> #4
good bug report, thanks! i'm assuming this isn't darwin-specific either --- it looks like we have similar logic using masks in the linux and windows backends and in the libusb backend too.
I'm not sure the switch to HandleError was a good idea, since it more or less swept the real problem under the rug.
yeah, i know what you mean, but it's also hard to argue with this logic in the commit message that made that change:
These CHECKs are expected to happen if the client does the wrong thing,
so we probably shouldn't be aborting in adbd.
a CHECK in the client (as you suggested earlier) would probably have been the best idea... postel's law and all that :-)
na...@google.com <na...@google.com> #5
Thanks for the quick response.
Indeed, I see similar logic in usb_write() in usb_linux. maxPacketSize is 512 (a power of 2) on my Ubuntu environment, so all good there as far as runtime behavior. It sure is odd that on macOS it's 5120, which happens to be exactly 10x the linux value. Seem suspicious, doesn't it?
Yeah, on the CHECK thing... I guess I see the client and server as a closed system in that the code is developed and distributed together. That said, obviously anyone can write their own adb client/server, so in that respect, I agree with the disagreement :-)
Indeed, I see similar logic in usb_write() in usb_linux. maxPacketSize is 512 (a power of 2) on my Ubuntu environment, so all good there as far as runtime behavior. It sure is odd that on macOS it's 5120, which happens to be exactly 10x the linux value. Seem suspicious, doesn't it?
Yeah, on the CHECK thing... I guess I see the client and server as a closed system in that the code is developed and distributed together. That said, obviously anyone can write their own adb client/server, so in that respect, I agree with the disagreement :-)
Description