Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-15 Thread Kinney, Michael D
Star, I agree they are optional. The description says DataLength should be 0 and Data should be NULL for the EfiUsbNoData case. That is why I did not think Direction needed to be evaluated if DataLength must be 0 in this case. In fact, the Host Controller implementations return an error if

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-15 Thread Zeng, Star
Mike, Yes, the DataLength could be read. And Data/DataLength is optional when Direction is EfiUsbNoData per my understanding. IN OUT VOID*Data, OPTIONAL IN UINTN DataLength, OPTIONAL Thanks, Star -Original Message- From: Kinney, Michael D

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-15 Thread Kinney, Michael D
Star, I have read that section, and I do not see anything that says the DataLength parameter can not be read. Mike > -Original Message- > From: Zeng, Star > Sent: Tuesday, November 14, 2017 8:23 PM > To: Kinney, Michael D ; > edk2-devel@lists.01.org > Cc:

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star
Mike, UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer(), in the description. Thanks, Star -Original Message- From: Kinney, Michael D Sent: Wednesday, November 15, 2017 12:00 PM To: Zeng, Star ; edk2-devel@lists.01.org; Kinney, Michael D

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Kinney, Michael D
Star, I am curious. Which spec statement are you referring to? Mike > -Original Message- > From: Zeng, Star > Sent: Tuesday, November 14, 2017 6:59 PM > To: Kinney, Michael D ; > edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star >

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star
Mike, I do not insist on the check that the Direction is not EfiUsbNoData, and I know the functionality should have no improvement with the check. So, you can have my Reviewed-by: Star Zeng . I raised the question just for consideration from literally, as according to the

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Kinney, Michael D
Star, For that case both DataLength and RequestedDataLength will be 0 and the new path will not be taken. Are you concerned that the USB HC Protocol could return a non zero DataLength for the EfiUsbNoData case? Even if it does, the new path can never be taken because 0 is less than all UINTN

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star
Mike, Do you think it is better or not to also check the Direction is not EfiUsbNoData? UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer(): If the Direction parameter is EfiUsbNoData, Data is NULL, and DataLength is 0, then no data phase exists for the control transfer. Thanks, Star

[edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Michael D Kinney
https://bugzilla.tianocore.org/show_bug.cgi?id=767 The USB I/O Protocol function ControlTransfer() has a DataLength parameter that specifies the size of the Data buffer. The UsbBusDxe module implements the USB I/O Protocol using the services of the USB2 Host Controller Protocol. The DataLength