On 02/14/20 01:55, Ni, Ray wrote:
-----Original Message-----Please don't misunderstand my points.
From: Laszlo Ersek <email@example.com>
Sent: Friday, February 14, 2020 7:15 AM
To: Ni, Ray <firstname.lastname@example.org>; email@example.com; Ard Biesheuvel
Cc: firstname.lastname@example.org; email@example.com; Gao, Zhichao
Subject: Re: [edk2-devel] [RFC PATCH 1/1] OvmfPkg: add 'initrd' shell
command to expose Linux initrd via device path
On 02/12/20 15:21, Ni, Ray wrote:
(3) However: I think this should be added as a Dynamic Command instead.
I'm basing this on the message of commit 0961002352e9 ("ShellPkg/tftp:
Convert from NULL class library to Dynamic Command", 2017-11-28),
is the first commit in edk2 ever to introduce a Dynamic Command.Laszlo, thanks for the comments😊.
And the commit message there says:
The guideline is:
1. Only use NULL class library for Shell spec defined commands.
2. New commands can be provided as not only a standalone application
but also a dynamic command. So it can be used either as an
internal command, but also as a standalone application.
I'm not asking for the command to be usable as a separate application,
but I think we might want to follow the first guideline.
(I've checked the UEFI Shell 2.2 spec. While it talks about dynamic
commands, it does not seem to spell out guideline#1. So I think it's
rather an edk2-specific guideline than a standard one. Nonetheless we
might want to adhere to it.)
I didn't remember that I said these guideline publicly.
The reason behind that is we can have the same shell binary everywhere
and new non-spec commands can be added through dynamic command
impacting the shell binary.Thanks for the explanation -- this means that the NULL class lib
approach is good for OvmfPkg after all. I'm putting the remaining parts
of this patch back on my review queue (it will take a while).
OK. From your response, I thought that the guidelines you captured in
the commit message in question were only for internal shell builds.
I still prefer to use dynamic commands
for all non-spec defined shell internal commands.
Sorry for the confusion caused by my previous mail.
It's OK, I understand better now. So I guess I'll de-queue the review of
the rest of this patch once again, and wait for the next version (with
the dynamic command implementation).