-----Original Message----- From: Marcin Wojtas [mailto:mw@semihalf.com] Sent: Wednesday, November 16, 2016 3:58 PM To: Carsey, Jaben jaben.carsey@intel.com Cc: Leif Lindholm leif.lindholm@linaro.org; Ard Biesheuvel ard.biesheuvel@linaro.org; linaro-uefi linaro-uefi@lists.linaro.org; Neta Zur Hershkovits neta@marvell.com; Yehuda Yitschak yehuday@marvell.com; Haim Boot hayim@marvell.com; Jan Dabros jsd@semihalf.com; Bartosz Szczepanek bsz@semihalf.com; edk2-devel-01 edk2-devel@lists.01.org; Ni, Ruiyu ruiyu.ni@intel.com Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell Importance: High
Hi Jaben,
I haven't found RunRegisteredCommand in newest edk2. Can you please point exactly what you meant?
I meant add a new function like that. It does not exist at all now.
As an alternative I checked ShellCommandRunCommandHandler - this is in fact a library helper function and it's nested deep down in RunShellCommand, which is for now the only working option. ShellCommandRunCommandHandler requires a lot of additional processing of the command line. All is done in Application/Shell code.
What's the difference that you need? I see this: looks like you would need a command line (which I assume you have), the return value (which I assume you want), and control over whether you want LastError changed based on this (I would suggest not, but...)
RETURN_STATUS EFIAPI ShellCommandRunCommandHandler ( IN CONST CHAR16 *CommandString, IN OUT SHELL_STATUS *RetVal, IN OUT BOOLEAN *CanAffectLE OPTIONAL )
The main difference between that and the RunShellCommand is things like file redirection, trimming space, alias replacement, and things that feel very "command-line" processing. I would think that your TFTP command line would be less needy of fixing up.
Is there any chance to expose RunShellCommand (or equivalent), so that it can be used in a nice way, not with including multi "../../" relative path header?
I expected, that the commands, which are in fact some wrappers or they mix multiple others is pretty much of a standard, I'm pretty surprised there are so huge difficulties in EDK2. I'm wondering of if there are any other options to be used here (unless we accept, what we have for now:) ).
I think we have less wrapping and less mixing that it appears at first. Those that are related are (I believe) in the same lib and as such, just call each other.
Best regards, Marcin
2016-11-16 22:48 GMT+01:00 Marcin Wojtas mw@semihalf.com:
Hi Jaben,
Thank you for your input.
I agree on EBL, but I have very little experience with EBL so I don’t want to
discuss in detail as I am not the right person without more research. Specifically, my gut reaction is that needing a platform specific boot loader indicates that something has already gone wrong on that platform.
However, this does not seem like a boot loader or an application at all. this is
an internal shell command. The goal here seems to be to create a NULL library to add a new internal command to the UEFI Shell. This library gets compiled/linked into the shell itself.
Indeed, it's nothing similar to the bootloader whatsoever. This command simply enables updating firmware in SPI flash directly from local path or from tftp.
I feel that we have found a "new" use case that I encountered, but worked
around in the past because all previous cases involved commands in the same library (there are interactions between Reconnect and Disconnect/Connect).
Right, however Reconnect is easy, as it simply calls gBS callbacks, whose definition are also in the same location.
I would say that a new API in the ShellCommandLib that links the UEFI Shell
Application to the NULL libraries that make up the internal commands would be my first choice for implementation. I would lean to something like the function that Marcin already called. Maybe this?
EFIAPI RunRegisteredCommand( CHAR16* CommandLine, EFI_STATUS *CommandReturnValue )
I'll try this one and let know.
Thanks, Marcin