Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
Reviewed-by: Ruiyu Ni
Thanks/Ray
> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 4:41 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben
> 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 ; Carsey, Jaben
> > > ; Ni, Ruiyu
> > > 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
> > > Cc: Ruiyu Ni
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Hao Wu
> > > ---
> > > 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [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 ; Carsey, Jaben
> > ; Ni, Ruiyu
> > 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
> > Cc: Ruiyu Ni
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu
> > ---
> > 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [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.
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 ; Carsey, Jaben
> ; Ni, Ruiyu
> 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
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu
> ---
> 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
