Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/UsbBusDxe: Avoid continuing on error path

["Wu, Hao A" <hao.a.wu@intel.com>]

Sorry, could you help to share more information on what issue is met for this proposed patch?

It seems to me that the change made below is to handle the case where:

  Status = HubApi->GetPortStatus (HubIf, Port, &PortState);

  if (EFI_ERROR (Status)) {
    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: failed to get state of port %d\n", Port));
    return Status;
  } 

GetPortStatus() returns EFI_ SUCCESS, but does not update the output parameter 'PortState'.
Could you help to check what causes 'PortState' not being updated after a success return from GetPortStatus()? Thanks in advance.

Also, one inline comment below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Rhodes
> Sent: Tuesday, September 20, 2022 9:14 PM
> To: devel@edk2.groups.io
> Cc: Rhodes, Sean <sean@starlabs.systems>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/UsbBusDxe: Avoid
> continuing on error path
> 
> Zero out the PortState in case GetPortStatus didn't set it, to avoid
> continuing with EFI_DEVICE_ERROR.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> ---
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> index aed34596f4..7fc567898a 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> @@ -900,6 +900,11 @@ UsbEnumeratePort (
>    Child  = NULL;
> 
>    HubApi = HubIf->HubApi;
> 
> 
> 
> +  // Zero out PortState in case GetPortStatus does not set it and we
> 
> +  // continue on the EFI_DEVICE_ERROR path
> 
> +  PortState.PortStatus       = 0;
> 
> +  PortState.PortChangeStatus = 0;


Could you help to use:

  ZeroMem (&PortState, sizeof (EFI_USB_PORT_STATUS));

here to align with other places in this driver?


Best Regards,
Hao Wu


> 
> +
> 
>    //
> 
>    // Host learns of the new device by polling the hub for port changes.
> 
>    //
> 
> --
> 2.34.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#93989): https://edk2.groups.io/g/devel/message/93989
> Mute This Topic: https://groups.io/mt/93802896/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
>