EDK2 (UEFI) Shell issues/bugs?

[mhaeuser]

Aloha,

 

For quite some while, the EDK2 UEFI Shell had a very bad reputation for being buggy or not even working at all on quite a few systems. Recently, the issue regarding the SHIFT modifier (where the cursor would be re-set to the beginning of the input) has been fixed by an EDK2 contributor. As I was curious whether or not the EDK2 Shell is finally in a useful state, I tried it out and saw it wouldn't boot. The issue was that they quieried only one of two possible GUIDs for a protocol. Furthermore, I tried the 'bcfg' command and saw that the 'addp' operation worked the same way as 'add', which I also fixed by some slight code editing. Both of those patches have been submited to edk2-devel and are open for review.

 

Now my question, does anyone of you still - or in the past have - experience any issues with the EDK2 Shell? If so, I would ask you to try the latest EDK2 Shell with the two attached patches and report any bugs left. I would really like to permanently switch to using (and developing for?) the EDK2 Shell once it has been reported to work alright for anyone... it did for me.

 

Attached is the latest EDK2 Shell with the patches below applied and, I think, some additional ASSERTs.

 

Greets,

DF.

 

 

ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
index b407608d3154..bf35a35b985d 100644
--- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
+++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
@@ -310,6 +310,7 @@ BcfgAdd(
{
EFI_STATUS Status;
EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ EFI_DEVICE_PATH_PROTOCOL *DevPath;
EFI_DEVICE_PATH_PROTOCOL *FilePath;
CHAR16 *Str;
UINT8 *TempByteBuffer;
@@ -462,9 +463,9 @@ BcfgAdd(
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_BCFG_FILE_DP), gShellBcfgHiiHandle, L"bcfg", Arg->FullName);
ShellStatus = SHELL_UNSUPPORTED;
} else {
-/*
if (UsePath) {
- DevPath = DevicePath;
+ DevPath = DevicePath;
+ ShellStatus = SHELL_INVALID_PARAMETER;
while (!IsDevicePathEnd(DevPath)) {
if ((DevicePathType(DevPath) == MEDIA_DEVICE_PATH) &&
(DevicePathSubType(DevPath) == MEDIA_HARDDRIVE_DP)) {
@@ -472,24 +473,15 @@ BcfgAdd(
//
// If we find it use it instead
//
- DevicePath = DevPath;
+ ShellStatus = SHELL_SUCCESS;
+ FilePath = DuplicateDevicePath (DevPath);
break;
}
DevPath = NextDevicePathNode(DevPath);
}
- //
- // append the file
- //
- for(StringWalker=Arg->FullName; *StringWalker != CHAR_NULL && *StringWalker != ':'; StringWalker++);
- FileNode = FileDevicePath(NULL, StringWalker+1);
- FilePath = AppendDevicePath(DevicePath, FileNode);
- FreePool(FileNode);
} else {
-*/
FilePath = DuplicateDevicePath(DevicePath);
-/*
}
-*/
FreePool(DevicePath);
}
}
--

 

 

 

---
ShellPkg/Application/Shell/Shell.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 47b3118ea701..2c564090721d 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -904,12 +904,19 @@ ProcessCommandLine(
// like a shell option (which is assumed to be `file-name`).

Status = gBS->LocateProtocol (
- &gEfiUnicodeCollationProtocolGuid,
+ &gEfiUnicodeCollation2ProtocolGuid,
NULL,
(VOID **) &UnicodeCollation
);
if (EFI_ERROR (Status)) {
- return Status;
+ Status = gBS->LocateProtocol (
+ &gEfiUnicodeCollationProtocolGuid,
+ NULL,
+ (VOID **) &UnicodeCollation
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}

// Set default options
--

 

Shell.efi

[Rocky12]

very good but I lack the experience to test

Edited May 20, 2016 by Allan
English please

[cecekpawon]

With amount of people downloading your attachment, I believe lot of them must be interest if you (re)develop this project / deliver some potential fixes. Yesterday, I add an options to (Clover) user to replace current Shell.efi from their repo with the one from deprecated "EdkShellBinPkg" bin (poor without some unix cmd, seems the base of HermitShellX64 with some advantages: bcfg, bdmesg, nvram, ...). Bin from "ShellBinPkg" (which just hang on start). Yours can boot fine, but always exit with assert, while Hermit always hang on exit from "edit" (sometime rendered bigger, stretched including fonts, but never fullscreen). All "edit" cmd cannot read plain plist properly (I dont know, maybe "\t" / <tag> / Unicode encoding may caused that problem), but I loved the old interface. "mv" sometimes broken. Cannot see cursor while typing: with yours / self compile last edk2 commits without your patches. Rest, will let more advanced users to do more tests. Good luck @DF

[Zenith432]

These patches have been committed to EDK2 in github - but ShellBinPkg not rebuilt yet.

 

cecekpawon - Your problems with shell from ShellBinPkg are not universal.  The newer shell for me has always worked, though may be bugs in it that I didn't notice in features I don't use.

It could be problem in the Windows build.  For instance, the shell converts device-paths to text on startup (for the map), which can cause potential hangs with use of va_list.

[cecekpawon]

Yes Zenith, will do test after they update their ShellBinPkg. Sorry to mention, but Slice appear to update "Patches_for_EDK2/ShellPkg" last time but forgot to update binary?

 

* Zenith432: hate to says, but compile with my hack both GCC & XCODE just boot fine.. Thanks! Windows is rotting alone

** Confirm: by using @DF "Shell.c" patch, now it can boot, uhm

Edited May 20, 2016 by cecekpawon

[mhaeuser]

ASSERT on exit is normal, someone put an "ASSERT (FALSE)" because of a memory leak for whatever reason...

[Slice]

If we propose return to Clover (not account OZ users  ) the ASSERT on exit is unreliable.

[mhaeuser]

What do Oz and Clover have to do with an UEFI Shell and its ASSERT() calls?

 

If you mean exiting will be interrupted, then this is for once not exclusive to Clover and won't bother any users as ASSERT() is DEBUG-only.

[cecekpawon]

All "edit" cmd cannot read plain plist properly (I dont know, maybe "\t" / <tag> / Unicode encoding may caused that problem)...

 

Confirmed Plistedit pro always create "tab" as indentation, which shell edit cannot handle this. How can I convert tab to space direct from shell? Search&replace from shell edit didnt help, and hexedit doesnt have this options..

[mhaeuser]

Are you sure the isse is tab vs space rather than CRLF vs LF?

[cecekpawon]

Yes @DF, please check attached plist examples

[cecekpawon]

How are you sir? Just want to report, i dont have tabbed indentation issues on virtual machine.. strange

[davidm71]

Anyone know how to inject the shell efi file directly into ones Uefi bios and have it accessible from a control key or boot menu item? I want to mod certain bios roms to have this feature kind of like how it is with Ozmosis and hermit shell but without Oz installed. Anyone please shed light on this?

 

 

Thanks

[cecekpawon]

Confirmed Plistedit pro always create "tab" as indentation, which shell edit cannot handle this. How can I convert tab to space direct from shell? Search&replace from shell edit didnt help, and hexedit doesnt have this options..

 

So I finally create a small shell util to convert tabs to spaces:

* Usage: shellexpand -t[:n] infile outfile
         shellexpand infile outfile
         shellexpand inoutfile
         shellexpand -h
* Default / min tab size (-t): 2

ShellExpand.efi.zip

[Slice]

Confirmed Plistedit pro always create "tab" as indentation, which shell edit cannot handle this. How can I convert tab to space direct from shell? Search&replace from shell edit didnt help, and hexedit doesnt have this options..

            #86            

[cecekpawon]

Yosh Slice, did some test with your patched version and its working great here for reading & writing back to file.. Thanks sergey

Assumed the remote path will never get change in the future, here the link for quick access to the binary >>>

Edited November 8, 2017 by cecekpawon

[Badruzeus]

This one is working fine for me (Asus: AMI Aptio 2.0.1) on UEFI Firmware, thanks @vit9696

I just need to rename it to "SHELLX64.EFI" on root of ESP for loading it through BIOS, or "/EFI/CLOVER/tools/Shell64U.efi" via Clover. Du no why EDK II Shell provided by Clover won't work (BestLanguage) even with "/drivers64UEFI/EnglishDxe-64.efi" installed. Another one that works is this one (from old Clover rev, I think).

 

My question is; How to include https://github.com/acidanthera/OpenCoreShell into Clover package so.. I don't need to manually replace it on every Clover post-install? Thanks.