In my opinion, ASSERT in this mandatory lib is fine. We have the depex of the protocol, that means the three protocols should have been installed during boot. Then the ASSERT would be a critical bug because of the failure of running Boot Services.

Thanks,

Zhichao

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov via Groups.Io
Sent: Wednesday, December 18, 2019 4:27 PM
To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances

This makes very good sense to me, thank you for taking your time to fix it.

I am slightly unsure whether if checks with subsequent assertions are really needed in mandatory version, as asserting in the constructor will trigger missing protocol very early anyway, but I do not think it is important.

Best wishes,

Vitaly

On Wed, Dec 18, 2019 at 05:10, Zhichao Gao <zhichao.gao@intel.com> wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2298

The UefiDevicePathLibOptionalDevicePathProtocolConstructor's implementation
isn't match with its instance name.
Remove the ASSERT and depex of the gEfiDevicePathUtilitiesProtocolGuid
because of "Optional".

Add a mandatory instance to force using the DevicePathUtilities,
DevicePathToText and DevicePathFromText protocol with the ASSERT
and depex.

V2:
The optional lib instance's construction should return success all the
time.
Change the desciption of the optional lib uni file.
Change the copyright date of the mandatory one's uni file.

V3:
Remove the Status variable in UefiDevicePathLibOptionalDevicePathProtocolConstructor.
The Status would cause GCC build fail because the variable is initialized but not used.
Since it is useless for the constructor, directly remove it.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>

Zhichao Gao (2):
MdePkg/UefiDevicePathLib: Separate the device path lib
MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build

...DevicePathLibMandatoryDevicePathProtocol.c | 469 ++++++++++++++++++
...vicePathLibMandatoryDevicePathProtocol.inf | 86 ++++
...vicePathLibMandatoryDevicePathProtocol.uni | 18 +
...iDevicePathLibOptionalDevicePathProtocol.c | 21 +-
...evicePathLibOptionalDevicePathProtocol.inf | 5 +-
...evicePathLibOptionalDevicePathProtocol.uni | 6 +-
MdePkg/MdePkg.dsc | 3 +-
7 files changed, 587 insertions(+), 21 deletions(-)
create mode 100644 MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c
create mode 100644 MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
create mode 100644 MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni

--
2.21.0.windows.1