Re: [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()

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

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, November 14, 2017 4:38 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Carsey, Jaben
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> How about changing the function header comments from
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or the medium is no longer supported.
> 
> To:
> > +  @retval others  Error status returned from
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().
> 
> Because MEDIA_CHANGED is not the only error that could happen.

Yes, I will update the comments when I push the patch if there's no other
comments.


Best Regards,
Hao Wu

> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Tuesday, November 14, 2017 3:54 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
> > <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > OpenRootByHandle()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> >
> > For the API EfiShellOpenRootByHandle():
> >
> > The return status of the call to SimpleFileSystem->OpenVolume should be
> > checked.
> >
> > It is possible that there is a media change in the device (like CD/DVD ROM).
> In
> > such case, the volume root opened and/or the device path opened previously
> > (also within EfiShellOpenRootByHandle) may be invalid.
> >
> > This commit adds a check for the result of OpenVolume before subsequently
> > calling functions like EfiShellGetMapFromDevicePath() &
> > ConvertEfiFileProtocolToShellHandle().
> >
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> > b/ShellPkg/Application/Shell/ShellProtocol.c
> > index 5e34b8dad1..0dee267353 100644
> > --- a/ShellPkg/Application/Shell/ShellProtocol.c
> > +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> > @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
> >    @retval EFI_NOT_FOUND         EFI_SIMPLE_FILE_SYSTEM could not be
> found
> > or the root directory
> >                                  could not be opened.
> >    @retval EFI_VOLUME_CORRUPTED  The data structures in the volume
> were
> > corrupted.
> > -  @retval EFI_DEVICE_ERROR      The device had an error
> > +  @retval EFI_DEVICE_ERROR      The device had an error.
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or
> > the medium is no longer supported.
> >  **/
> >  EFI_STATUS
> >  EFIAPI
> > @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
> >    // Open the root volume now...
> >    //
> >    Status = SimpleFileSystem->OpenVolume(SimpleFileSystem,
> &RealFileHandle);
> > +  if (EFI_ERROR(Status)) {
> > +    return Status;
> > +  }
> > +
> >    *FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> > EfiShellGetMapFromDevicePath(&DevPath));
> > -  return (Status);
> > +  return (EFI_SUCCESS);
> >  }
> >
> >  /**
> > --
> > 2.12.0.windows.1