* [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor @ 2016-10-05 20:58 Tapan Shah 2016-10-05 22:20 ` Carsey, Jaben 0 siblings, 1 reply; 5+ messages in thread From: Tapan Shah @ 2016-10-05 20:58 UTC (permalink / raw) To: edk2-devel; +Cc: jaben.carsey, Tapan Shah Move gEfiUnicodeCollation2ProtocolGuid protocol outside of UefiShellLib constructor function. Locate gEfiUnicodeCollation2ProtocolGuid protocol in ShellOpenFileByName() which consumes this protocol API. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Tapan Shah <tapandshah@hpe.com> --- ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c index e47d535..53f54e1 100644 --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c @@ -292,18 +292,12 @@ ShellLibConstructor ( IN EFI_SYSTEM_TABLE *SystemTable ) { - EFI_STATUS Status; - mEfiShellEnvironment2 = NULL; gEfiShellProtocol = NULL; gEfiShellParametersProtocol = NULL; mEfiShellInterface = NULL; mEfiShellEnvironment2Handle = NULL; - - if (mUnicodeCollationProtocol == NULL) { - Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol); - ASSERT_EFI_ERROR (Status); - } + mUnicodeCollationProtocol = NULL; // // verify that auto initialize is not set false @@ -730,6 +724,14 @@ ShellOpenFileByName( FileHandle, OpenMode); + if (mUnicodeCollationProtocol == NULL) { + Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol); + if (EFI_ERROR (Status)) { + gEfiShellProtocol->CloseFile (*FileHandle); + return Status; + } + } + if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, (CHAR16*)FileName, L"NUL") != 0) && (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, (CHAR16*)FileName, L"NULL") != 0) && !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){ -- 1.9.5.msysgit.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor 2016-10-05 20:58 [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor Tapan Shah @ 2016-10-05 22:20 ` Carsey, Jaben 2016-10-06 19:57 ` Vladimir Olovyannikov 0 siblings, 1 reply; 5+ messages in thread From: Carsey, Jaben @ 2016-10-05 22:20 UTC (permalink / raw) To: Tapan Shah, edk2-devel@lists.01.org; +Cc: Carsey, Jaben And pushed. Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Tapan Shah [mailto:tapandshah@hpe.com] > Sent: Wednesday, October 05, 2016 1:58 PM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Tapan Shah > <tapandshah@hpe.com> > Subject: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of > UefiShellLib constructor > Importance: High > > Move gEfiUnicodeCollation2ProtocolGuid protocol outside of UefiShellLib > constructor > function. > Locate gEfiUnicodeCollation2ProtocolGuid protocol in > ShellOpenFileByName() which > consumes this protocol API. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Tapan Shah <tapandshah@hpe.com> > --- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index e47d535..53f54e1 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -292,18 +292,12 @@ ShellLibConstructor ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > - EFI_STATUS Status; > - > mEfiShellEnvironment2 = NULL; > gEfiShellProtocol = NULL; > gEfiShellParametersProtocol = NULL; > mEfiShellInterface = NULL; > mEfiShellEnvironment2Handle = NULL; > - > - if (mUnicodeCollationProtocol == NULL) { > - Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, > (VOID**)&mUnicodeCollationProtocol); > - ASSERT_EFI_ERROR (Status); > - } > + mUnicodeCollationProtocol = NULL; > > // > // verify that auto initialize is not set false > @@ -730,6 +724,14 @@ ShellOpenFileByName( > FileHandle, > OpenMode); > > + if (mUnicodeCollationProtocol == NULL) { > + Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, > NULL, (VOID**)&mUnicodeCollationProtocol); > + if (EFI_ERROR (Status)) { > + gEfiShellProtocol->CloseFile (*FileHandle); > + return Status; > + } > + } > + > if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NUL") != 0) && > (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NULL") != 0) && > !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){ > -- > 1.9.5.msysgit.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor 2016-10-05 22:20 ` Carsey, Jaben @ 2016-10-06 19:57 ` Vladimir Olovyannikov 2016-10-06 20:05 ` Shah, Tapan 0 siblings, 1 reply; 5+ messages in thread From: Vladimir Olovyannikov @ 2016-10-06 19:57 UTC (permalink / raw) To: Carsey, Jaben, Tapan Shah, edk2-devel Hi Carsey, Tapan, This change breaks our platform's UEFI Shell at runtime (no asserts) : ... Press ESC in 1 seconds to skip startup.nsh or any other key to continue. Error: Image at 000B97CF000 start failed: Invalid Parameter remove-symbol-file /uefi/Build/NS2Pkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shell/DEB UG/Shell.dll 0xB97CF000 Image Return Status = Invalid Parameter UnicodeCollation protocol is present. Attempt to open fs1:startup.nsh which is not present. There is a bug in UefiShellLib.c ShellOpenFileByName() which has nothing to do to this commit. 723 Status = gEfiShellProtocol->OpenFileByName(FileName, FileHandle, OpenMode); If there is no file available, the status is EFI_NOT_FOUND. Line 726 (or 727) of the UefiShellLib.c must be if (EFI_ERROR(Status)) { return Status; } But it is missing, and therefore EFI_SUCCESS is returned to the Shell.c from ShellFindFilePath() (line 1228 of Shell.c), which leads to an attempt to run a non-existent script which fails, and this leads to Shell unloading with "Invalid parameter". Thank you, Vladimir -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Carsey, Jaben Sent: Wednesday, October 05, 2016 3:20 PM To: Tapan Shah; edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: Re: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor And pushed. Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Tapan Shah [mailto:tapandshah@hpe.com] > Sent: Wednesday, October 05, 2016 1:58 PM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Tapan Shah > <tapandshah@hpe.com> > Subject: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of > UefiShellLib constructor > Importance: High > > Move gEfiUnicodeCollation2ProtocolGuid protocol outside of UefiShellLib > constructor > function. > Locate gEfiUnicodeCollation2ProtocolGuid protocol in > ShellOpenFileByName() which > consumes this protocol API. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Tapan Shah <tapandshah@hpe.com> > --- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index e47d535..53f54e1 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -292,18 +292,12 @@ ShellLibConstructor ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > - EFI_STATUS Status; > - > mEfiShellEnvironment2 = NULL; > gEfiShellProtocol = NULL; > gEfiShellParametersProtocol = NULL; > mEfiShellInterface = NULL; > mEfiShellEnvironment2Handle = NULL; > - > - if (mUnicodeCollationProtocol == NULL) { > - Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, > (VOID**)&mUnicodeCollationProtocol); > - ASSERT_EFI_ERROR (Status); > - } > + mUnicodeCollationProtocol = NULL; > > // > // verify that auto initialize is not set false > @@ -730,6 +724,14 @@ ShellOpenFileByName( > FileHandle, > OpenMode); > > + if (mUnicodeCollationProtocol == NULL) { > + Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, > NULL, (VOID**)&mUnicodeCollationProtocol); > + if (EFI_ERROR (Status)) { > + gEfiShellProtocol->CloseFile (*FileHandle); > + return Status; > + } > + } > + > if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NUL") != 0) && > (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NULL") != 0) && > !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){ > -- > 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor 2016-10-06 19:57 ` Vladimir Olovyannikov @ 2016-10-06 20:05 ` Shah, Tapan 2016-10-06 20:07 ` Vladimir Olovyannikov 0 siblings, 1 reply; 5+ messages in thread From: Shah, Tapan @ 2016-10-06 20:05 UTC (permalink / raw) To: Vladimir Olovyannikov, Carsey, Jaben, edk2-devel@lists.01.org Can you send a patch file with this error check added @726 ? -----Original Message----- From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com] Sent: Thursday, October 06, 2016 2:58 PM To: Carsey, Jaben <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor Hi Carsey, Tapan, This change breaks our platform's UEFI Shell at runtime (no asserts) : ... Press ESC in 1 seconds to skip startup.nsh or any other key to continue. Error: Image at 000B97CF000 start failed: Invalid Parameter remove-symbol-file /uefi/Build/NS2Pkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shell/DEB UG/Shell.dll 0xB97CF000 Image Return Status = Invalid Parameter UnicodeCollation protocol is present. Attempt to open fs1:startup.nsh which is not present. There is a bug in UefiShellLib.c ShellOpenFileByName() which has nothing to do to this commit. 723 Status = gEfiShellProtocol->OpenFileByName(FileName, FileHandle, OpenMode); If there is no file available, the status is EFI_NOT_FOUND. Line 726 (or 727) of the UefiShellLib.c must be if (EFI_ERROR(Status)) { return Status; } But it is missing, and therefore EFI_SUCCESS is returned to the Shell.c from ShellFindFilePath() (line 1228 of Shell.c), which leads to an attempt to run a non-existent script which fails, and this leads to Shell unloading with "Invalid parameter". Thank you, Vladimir -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Carsey, Jaben Sent: Wednesday, October 05, 2016 3:20 PM To: Tapan Shah; edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: Re: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor And pushed. Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Tapan Shah [mailto:tapandshah@hpe.com] > Sent: Wednesday, October 05, 2016 1:58 PM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Tapan Shah > <tapandshah@hpe.com> > Subject: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out > of UefiShellLib constructor > Importance: High > > Move gEfiUnicodeCollation2ProtocolGuid protocol outside of > UefiShellLib constructor function. > Locate gEfiUnicodeCollation2ProtocolGuid protocol in > ShellOpenFileByName() which > consumes this protocol API. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Tapan Shah <tapandshah@hpe.com> > --- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index e47d535..53f54e1 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -292,18 +292,12 @@ ShellLibConstructor ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > - EFI_STATUS Status; > - > mEfiShellEnvironment2 = NULL; > gEfiShellProtocol = NULL; > gEfiShellParametersProtocol = NULL; > mEfiShellInterface = NULL; > mEfiShellEnvironment2Handle = NULL; > - > - if (mUnicodeCollationProtocol == NULL) { > - Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, > (VOID**)&mUnicodeCollationProtocol); > - ASSERT_EFI_ERROR (Status); > - } > + mUnicodeCollationProtocol = NULL; > > // > // verify that auto initialize is not set false @@ -730,6 +724,14 > @@ ShellOpenFileByName( > FileHandle, > OpenMode); > > + if (mUnicodeCollationProtocol == NULL) { > + Status = gBS->LocateProtocol > + (&gEfiUnicodeCollation2ProtocolGuid, > NULL, (VOID**)&mUnicodeCollationProtocol); > + if (EFI_ERROR (Status)) { > + gEfiShellProtocol->CloseFile (*FileHandle); > + return Status; > + } > + } > + > if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NUL") != 0) && > (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NULL") != 0) && > !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){ > -- > 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor 2016-10-06 20:05 ` Shah, Tapan @ 2016-10-06 20:07 ` Vladimir Olovyannikov 0 siblings, 0 replies; 5+ messages in thread From: Vladimir Olovyannikov @ 2016-10-06 20:07 UTC (permalink / raw) To: Shah, Tapan, Carsey, Jaben, edk2-devel OK, will do -----Original Message----- From: Shah, Tapan [mailto:tapandshah@hpe.com] Sent: Thursday, October 06, 2016 1:05 PM To: Vladimir Olovyannikov; Carsey, Jaben; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor Can you send a patch file with this error check added @726 ? -----Original Message----- From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com] Sent: Thursday, October 06, 2016 2:58 PM To: Carsey, Jaben <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor Hi Carsey, Tapan, This change breaks our platform's UEFI Shell at runtime (no asserts) : ... Press ESC in 1 seconds to skip startup.nsh or any other key to continue. Error: Image at 000B97CF000 start failed: Invalid Parameter remove-symbol-file /uefi/Build/NS2Pkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shell/DEB UG/Shell.dll 0xB97CF000 Image Return Status = Invalid Parameter UnicodeCollation protocol is present. Attempt to open fs1:startup.nsh which is not present. There is a bug in UefiShellLib.c ShellOpenFileByName() which has nothing to do to this commit. 723 Status = gEfiShellProtocol->OpenFileByName(FileName, FileHandle, OpenMode); If there is no file available, the status is EFI_NOT_FOUND. Line 726 (or 727) of the UefiShellLib.c must be if (EFI_ERROR(Status)) { return Status; } But it is missing, and therefore EFI_SUCCESS is returned to the Shell.c from ShellFindFilePath() (line 1228 of Shell.c), which leads to an attempt to run a non-existent script which fails, and this leads to Shell unloading with "Invalid parameter". Thank you, Vladimir -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Carsey, Jaben Sent: Wednesday, October 05, 2016 3:20 PM To: Tapan Shah; edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: Re: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor And pushed. Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Tapan Shah [mailto:tapandshah@hpe.com] > Sent: Wednesday, October 05, 2016 1:58 PM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Tapan Shah > <tapandshah@hpe.com> > Subject: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out > of UefiShellLib constructor > Importance: High > > Move gEfiUnicodeCollation2ProtocolGuid protocol outside of > UefiShellLib constructor function. > Locate gEfiUnicodeCollation2ProtocolGuid protocol in > ShellOpenFileByName() which > consumes this protocol API. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Tapan Shah <tapandshah@hpe.com> > --- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index e47d535..53f54e1 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -292,18 +292,12 @@ ShellLibConstructor ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > - EFI_STATUS Status; > - > mEfiShellEnvironment2 = NULL; > gEfiShellProtocol = NULL; > gEfiShellParametersProtocol = NULL; > mEfiShellInterface = NULL; > mEfiShellEnvironment2Handle = NULL; > - > - if (mUnicodeCollationProtocol == NULL) { > - Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, > (VOID**)&mUnicodeCollationProtocol); > - ASSERT_EFI_ERROR (Status); > - } > + mUnicodeCollationProtocol = NULL; > > // > // verify that auto initialize is not set false @@ -730,6 +724,14 > @@ ShellOpenFileByName( > FileHandle, > OpenMode); > > + if (mUnicodeCollationProtocol == NULL) { > + Status = gBS->LocateProtocol > + (&gEfiUnicodeCollation2ProtocolGuid, > NULL, (VOID**)&mUnicodeCollationProtocol); > + if (EFI_ERROR (Status)) { > + gEfiShellProtocol->CloseFile (*FileHandle); > + return Status; > + } > + } > + > if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NUL") != 0) && > (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, > (CHAR16*)FileName, L"NULL") != 0) && > !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){ > -- > 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-06 20:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-05 20:58 [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor Tapan Shah 2016-10-05 22:20 ` Carsey, Jaben 2016-10-06 19:57 ` Vladimir Olovyannikov 2016-10-06 20:05 ` Shah, Tapan 2016-10-06 20:07 ` Vladimir Olovyannikov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox