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

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 4:41 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> > -----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