public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
@ 2019-12-18  2:10 Gao, Zhichao
  2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-18  2:10 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Vitaly Cheptsov

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib
  2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
@ 2019-12-18  2:10 ` Gao, Zhichao
  2019-12-18  8:27   ` Vitaly Cheptsov
  2019-12-18  2:10 ` [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build Gao, Zhichao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-18  2:10 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Vitaly Cheptsov

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

UefiDevicePathLibOptionalDevicePathProtocol's implementation isn't
fit its description. It should be implement as blow:
Try to find the DevicePathProtocol, if found then use it to implement
the interface. Else, use the local interface. It should not have the
depex and ASSERT of gEfiDevicePathUtilitiesProtocolGuid when not find
the DevicePathProtocol.

Add a mandatory one to force using the DevicePathUtilities,
DevicePathToText and DevicePathFromText protocol.

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

V2:
Optional one should always return EFI_SUCCESS in its constructor.
Change the description of optional one's uni file.
Fix the copyright date of 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.

 ...DevicePathLibMandatoryDevicePathProtocol.c | 469 ++++++++++++++++++
 ...vicePathLibMandatoryDevicePathProtocol.inf |  86 ++++
 ...vicePathLibMandatoryDevicePathProtocol.uni |  18 +
 ...iDevicePathLibOptionalDevicePathProtocol.c |  21 +-
 ...evicePathLibOptionalDevicePathProtocol.inf |   5 +-
 ...evicePathLibOptionalDevicePathProtocol.uni |   6 +-
 6 files changed, 585 insertions(+), 20 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

diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c
new file mode 100644
index 0000000000..fa27110fd4
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c
@@ -0,0 +1,469 @@
+/** @file
+  Device Path services. The thing to remember is device paths are built out of
+  nodes. The device path is terminated by an end node that is length
+  sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is sizeof(EFI_DEVICE_PATH_PROTOCOL)
+  all over this file.
+
+  The only place where multi-instance device paths are supported is in
+  environment varibles. Multi-instance device paths should never be placed
+  on a Handle.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include "UefiDevicePathLib.h"
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_UTILITIES_PROTOCOL *mDevicePathLibDevicePathUtilities = NULL;
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_TO_TEXT_PROTOCOL   *mDevicePathLibDevicePathToText    = NULL;
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *mDevicePathLibDevicePathFromText  = NULL;
+
+/**
+  The constructor function caches the pointer to DevicePathUtilites protocol,
+  DevicePathToText protocol and DevicePathFromText protocol.
+
+  The constructor function locates these three protocols from protocol database.
+  It will caches the pointer to local protocol instance if that operation fails
+  and it will always return EFI_SUCCESS.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+UefiDevicePathLibMandatoryDevicePathProtocolConstructor (
+  IN      EFI_HANDLE                ImageHandle,
+  IN      EFI_SYSTEM_TABLE          *SystemTable
+  )
+{
+  EFI_STATUS                        Status;
+
+  Status = gBS->LocateProtocol (
+                  &gEfiDevicePathUtilitiesProtocolGuid,
+                  NULL,
+                  (VOID**) &mDevicePathLibDevicePathUtilities
+                  );
+  ASSERT_EFI_ERROR (Status);
+  ASSERT (mDevicePathLibDevicePathUtilities != NULL);
+
+  Status = gBS->LocateProtocol (
+                  &gEfiDevicePathToTextProtocolGuid,
+                  NULL,
+                  (VOID**) &mDevicePathLibDevicePathToText
+                  );
+  ASSERT_EFI_ERROR (Status);
+  ASSERT (mDevicePathLibDevicePathToText != NULL);
+
+  Status = gBS->LocateProtocol (
+                  &gEfiDevicePathFromTextProtocolGuid,
+                  NULL,
+                  (VOID**) &mDevicePathLibDevicePathFromText
+                  );
+  ASSERT_EFI_ERROR (Status);
+  ASSERT (mDevicePathLibDevicePathFromText != NULL);
+
+  return Status;
+}
+
+/**
+  Returns the size of a device path in bytes.
+
+  This function returns the size, in bytes, of the device path data structure
+  specified by DevicePath including the end of device path node.
+  If DevicePath is NULL or invalid, then 0 is returned.
+
+  @param  DevicePath  A pointer to a device path data structure.
+
+  @retval 0           If DevicePath is NULL or invalid.
+  @retval Others      The size of a device path in bytes.
+
+**/
+UINTN
+EFIAPI
+GetDevicePathSize (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->GetDevicePathSize (DevicePath);
+  }
+
+  ASSERT (FALSE);
+  return 0;
+}
+
+/**
+  Creates a new copy of an existing device path.
+
+  This function allocates space for a new copy of the device path specified by DevicePath.
+  If DevicePath is NULL, then NULL is returned.  If the memory is successfully
+  allocated, then the contents of DevicePath are copied to the newly allocated
+  buffer, and a pointer to that buffer is returned.  Otherwise, NULL is returned.
+  The memory for the new device path is allocated from EFI boot services memory.
+  It is the responsibility of the caller to free the memory allocated.
+
+  @param  DevicePath    A pointer to a device path data structure.
+
+  @retval NULL          DevicePath is NULL or invalid.
+  @retval Others        A pointer to the duplicated device path.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DuplicateDevicePath (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->DuplicateDevicePath (DevicePath);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Creates a new device path by appending a second device path to a first device path.
+
+  This function creates a new device path by appending a copy of SecondDevicePath
+  to a copy of FirstDevicePath in a newly allocated buffer.  Only the end-of-device-path
+  device node from SecondDevicePath is retained. The newly created device path is
+  returned. If FirstDevicePath is NULL, then it is ignored, and a duplicate of
+  SecondDevicePath is returned.  If SecondDevicePath is NULL, then it is ignored,
+  and a duplicate of FirstDevicePath is returned. If both FirstDevicePath and
+  SecondDevicePath are NULL, then a copy of an end-of-device-path is returned.
+
+  If there is not enough memory for the newly allocated buffer, then NULL is returned.
+  The memory for the new device path is allocated from EFI boot services memory.
+  It is the responsibility of the caller to free the memory allocated.
+
+  @param  FirstDevicePath            A pointer to a device path data structure.
+  @param  SecondDevicePath           A pointer to a device path data structure.
+
+  @retval NULL      If there is not enough memory for the newly allocated buffer.
+  @retval NULL      If FirstDevicePath or SecondDevicePath is invalid.
+  @retval Others    A pointer to the new device path if success.
+                    Or a copy an end-of-device-path if both FirstDevicePath and SecondDevicePath are NULL.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+AppendDevicePath (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *FirstDevicePath,  OPTIONAL
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *SecondDevicePath  OPTIONAL
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->AppendDevicePath (FirstDevicePath, SecondDevicePath);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Creates a new path by appending the device node to the device path.
+
+  This function creates a new device path by appending a copy of the device node
+  specified by DevicePathNode to a copy of the device path specified by DevicePath
+  in an allocated buffer. The end-of-device-path device node is moved after the
+  end of the appended device node.
+  If DevicePathNode is NULL then a copy of DevicePath is returned.
+  If DevicePath is NULL then a copy of DevicePathNode, followed by an end-of-device
+  path device node is returned.
+  If both DevicePathNode and DevicePath are NULL then a copy of an end-of-device-path
+  device node is returned.
+  If there is not enough memory to allocate space for the new device path, then
+  NULL is returned.
+  The memory is allocated from EFI boot services memory. It is the responsibility
+  of the caller to free the memory allocated.
+
+  @param  DevicePath                 A pointer to a device path data structure.
+  @param  DevicePathNode             A pointer to a single device path node.
+
+  @retval NULL      If there is not enough memory for the new device path.
+  @retval Others    A pointer to the new device path if success.
+                    A copy of DevicePathNode followed by an end-of-device-path node
+                    if both FirstDevicePath and SecondDevicePath are NULL.
+                    A copy of an end-of-device-path node if both FirstDevicePath
+                    and SecondDevicePath are NULL.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+AppendDevicePathNode (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath,     OPTIONAL
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePathNode  OPTIONAL
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->AppendDeviceNode (DevicePath, DevicePathNode);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Creates a new device path by appending the specified device path instance to the specified device
+  path.
+
+  This function creates a new device path by appending a copy of the device path
+  instance specified by DevicePathInstance to a copy of the device path specified
+  by DevicePath in a allocated buffer.
+  The end-of-device-path device node is moved after the end of the appended device
+  path instance and a new end-of-device-path-instance node is inserted between.
+  If DevicePath is NULL, then a copy if DevicePathInstance is returned.
+  If DevicePathInstance is NULL, then NULL is returned.
+  If DevicePath or DevicePathInstance is invalid, then NULL is returned.
+  If there is not enough memory to allocate space for the new device path, then
+  NULL is returned.
+  The memory is allocated from EFI boot services memory. It is the responsibility
+  of the caller to free the memory allocated.
+
+  @param  DevicePath                 A pointer to a device path data structure.
+  @param  DevicePathInstance         A pointer to a device path instance.
+
+  @return A pointer to the new device path.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+AppendDevicePathInstance (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath,        OPTIONAL
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePathInstance OPTIONAL
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->AppendDevicePathInstance (DevicePath, DevicePathInstance);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Creates a copy of the current device path instance and returns a pointer to the next device path
+  instance.
+
+  This function creates a copy of the current device path instance. It also updates
+  DevicePath to point to the next device path instance in the device path (or NULL
+  if no more) and updates Size to hold the size of the device path instance copy.
+  If DevicePath is NULL, then NULL is returned.
+  If DevicePath points to a invalid device path, then NULL is returned.
+  If there is not enough memory to allocate space for the new device path, then
+  NULL is returned.
+  The memory is allocated from EFI boot services memory. It is the responsibility
+  of the caller to free the memory allocated.
+  If Size is NULL, then ASSERT().
+
+  @param  DevicePath                 On input, this holds the pointer to the current
+                                     device path instance. On output, this holds
+                                     the pointer to the next device path instance
+                                     or NULL if there are no more device path
+                                     instances in the device path pointer to a
+                                     device path data structure.
+  @param  Size                       On output, this holds the size of the device
+                                     path instance, in bytes or zero, if DevicePath
+                                     is NULL.
+
+  @return A pointer to the current device path instance.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+GetNextDevicePathInstance (
+  IN OUT EFI_DEVICE_PATH_PROTOCOL    **DevicePath,
+  OUT UINTN                          *Size
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->GetNextDevicePathInstance (DevicePath, Size);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Creates a device node.
+
+  This function creates a new device node in a newly allocated buffer of size
+  NodeLength and initializes the device path node header with NodeType and NodeSubType.
+  The new device path node is returned.
+  If NodeLength is smaller than a device path header, then NULL is returned.
+  If there is not enough memory to allocate space for the new device path, then
+  NULL is returned.
+  The memory is allocated from EFI boot services memory. It is the responsibility
+  of the caller to free the memory allocated.
+
+  @param  NodeType                   The device node type for the new device node.
+  @param  NodeSubType                The device node sub-type for the new device node.
+  @param  NodeLength                 The length of the new device node.
+
+  @return The new device path.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+CreateDeviceNode (
+  IN UINT8                           NodeType,
+  IN UINT8                           NodeSubType,
+  IN UINT16                          NodeLength
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->CreateDeviceNode (NodeType, NodeSubType, NodeLength);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Determines if a device path is single or multi-instance.
+
+  This function returns TRUE if the device path specified by DevicePath is
+  multi-instance.
+  Otherwise, FALSE is returned.
+  If DevicePath is NULL or invalid, then FALSE is returned.
+
+  @param  DevicePath                 A pointer to a device path data structure.
+
+  @retval  TRUE                      DevicePath is multi-instance.
+  @retval  FALSE                     DevicePath is not multi-instance, or DevicePath
+                                     is NULL or invalid.
+
+**/
+BOOLEAN
+EFIAPI
+IsDevicePathMultiInstance (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  )
+{
+  if (mDevicePathLibDevicePathUtilities != NULL) {
+    return mDevicePathLibDevicePathUtilities->IsDevicePathMultiInstance (DevicePath);
+  }
+
+  ASSERT (FALSE);
+  return FALSE;
+}
+
+/**
+  Converts a device node to its string representation.
+
+  @param DeviceNode        A Pointer to the device node to be converted.
+  @param DisplayOnly       If DisplayOnly is TRUE, then the shorter text representation
+                           of the display node is used, where applicable. If DisplayOnly
+                           is FALSE, then the longer text representation of the display node
+                           is used.
+  @param AllowShortcuts    If AllowShortcuts is TRUE, then the shortcut forms of text
+                           representation for a device node can be used, where applicable.
+
+  @return A pointer to the allocated text representation of the device node or NULL if DeviceNode
+          is NULL or there was insufficient memory.
+
+**/
+CHAR16 *
+EFIAPI
+ConvertDeviceNodeToText (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DeviceNode,
+  IN BOOLEAN                         DisplayOnly,
+  IN BOOLEAN                         AllowShortcuts
+  )
+{
+  if (mDevicePathLibDevicePathToText != NULL) {
+    return mDevicePathLibDevicePathToText->ConvertDeviceNodeToText (DeviceNode, DisplayOnly, AllowShortcuts);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Converts a device path to its text representation.
+
+  @param DevicePath      A Pointer to the device to be converted.
+  @param DisplayOnly     If DisplayOnly is TRUE, then the shorter text representation
+                         of the display node is used, where applicable. If DisplayOnly
+                         is FALSE, then the longer text representation of the display node
+                         is used.
+  @param AllowShortcuts  If AllowShortcuts is TRUE, then the shortcut forms of text
+                         representation for a device node can be used, where applicable.
+
+  @return A pointer to the allocated text representation of the device path or
+          NULL if DeviceNode is NULL or there was insufficient memory.
+
+**/
+CHAR16 *
+EFIAPI
+ConvertDevicePathToText (
+  IN CONST EFI_DEVICE_PATH_PROTOCOL   *DevicePath,
+  IN BOOLEAN                          DisplayOnly,
+  IN BOOLEAN                          AllowShortcuts
+  )
+{
+  if (mDevicePathLibDevicePathToText != NULL) {
+    return mDevicePathLibDevicePathToText->ConvertDevicePathToText (DevicePath, DisplayOnly, AllowShortcuts);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Convert text to the binary representation of a device node.
+
+  @param TextDeviceNode  TextDeviceNode points to the text representation of a device
+                         node. Conversion starts with the first character and continues
+                         until the first non-device node character.
+
+  @return A pointer to the EFI device node or NULL if TextDeviceNode is NULL or there was
+          insufficient memory or text unsupported.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+ConvertTextToDeviceNode (
+  IN CONST CHAR16 *TextDeviceNode
+  )
+{
+  if (mDevicePathLibDevicePathFromText != NULL) {
+    return mDevicePathLibDevicePathFromText->ConvertTextToDeviceNode (TextDeviceNode);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  Convert text to the binary representation of a device path.
+
+
+  @param TextDevicePath  TextDevicePath points to the text representation of a device
+                         path. Conversion starts with the first character and continues
+                         until the first non-device node character.
+
+  @return A pointer to the allocated device path or NULL if TextDeviceNode is NULL or
+          there was insufficient memory.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+ConvertTextToDevicePath (
+  IN CONST CHAR16 *TextDevicePath
+  )
+{
+  if (mDevicePathLibDevicePathFromText != NULL) {
+    return mDevicePathLibDevicePathFromText->ConvertTextToDevicePath (TextDevicePath);
+  }
+
+  ASSERT (FALSE);
+  return NULL;
+}
+
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
new file mode 100644
index 0000000000..eb545d4601
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
@@ -0,0 +1,86 @@
+## @file
+# Instance of Device Path Library based on Device Path Protocol.
+#
+#  Device Path Library that layers on top of the UEFI 2.0 Device Path Protocol.
+#  If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
+#  uses its internal conversion logic.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = UefiDevicePathLibMandatoryDevicePathProtocol
+  MODULE_UNI_FILE                = UefiDevicePathLibMandatoryDevicePathProtocol.uni
+  FILE_GUID                      = 5A8389AF-DAE4-407E-8FD5-D026948BC579
+  MODULE_TYPE                    = UEFI_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = DevicePathLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE
+
+  CONSTRUCTOR                    = UefiDevicePathLibMandatoryDevicePathProtocolConstructor
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+
+[Sources]
+  DevicePathUtilities.c
+  DevicePathToText.c
+  DevicePathFromText.c
+  UefiDevicePathLibMandatoryDevicePathProtocol.c
+  UefiDevicePathLib.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+
+[LibraryClasses]
+  BaseLib
+  UefiBootServicesTableLib
+  MemoryAllocationLib
+  DebugLib
+  BaseMemoryLib
+  PcdLib
+  PrintLib
+
+[Guids]
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiVTUTF8Guid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiVT100Guid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiVT100PlusGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiPcAnsiGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiUartDevicePathGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiSasDevicePathGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiVirtualDiskGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiVirtualCdGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiPersistentVirtualDiskGuid
+  ## SOMETIMES_CONSUMES  ## GUID
+  gEfiPersistentVirtualCdGuid
+
+[Protocols]
+  gEfiDevicePathProtocolGuid                    ## SOMETIMES_CONSUMES
+  gEfiDevicePathUtilitiesProtocolGuid           ## CONSUMES
+  gEfiDevicePathToTextProtocolGuid              ## SOMETIMES_CONSUMES
+  gEfiDevicePathFromTextProtocolGuid            ## SOMETIMES_CONSUMES
+  gEfiDebugPortProtocolGuid                     ## UNDEFINED
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ## SOMETIMES_CONSUMES
+
+[Depex.common.DXE_DRIVER, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SAL_DRIVER, Depex.common.DXE_SMM_DRIVER]
+  gEfiDevicePathUtilitiesProtocolGuid AND
+  gEfiDevicePathToTextProtocolGuid AND
+  gEfiDevicePathFromTextProtocolGuid
+
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni
new file mode 100644
index 0000000000..1b8a556698
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni
@@ -0,0 +1,18 @@
+// /** @file
+// Instance of Device Path Library based on Device Path Protocol.
+//
+// Device Path Library that layers on top of the UEFI 2.0 Device Path Protocol.
+// If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
+// uses its internal conversion logic.
+//
+// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Instance of Device Path Library based on Device Path Protocol."
+
+#string STR_MODULE_DESCRIPTION          #language en-US "Instance of Device Path Library based on Device Path Protocol."
+
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
index 106ff245cc..ddef75da18 100644
--- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
@@ -8,7 +8,7 @@
   environment varibles. Multi-instance device paths should never be placed
   on a Handle.
 
-  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -21,8 +21,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_TO_TEXT_PROTOCOL   *mDevicePathLib
 GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *mDevicePathLibDevicePathFromText  = NULL;
 
 /**
-  The constructor function caches the pointer to DevicePathUtilites protocol,
-  DevicePathToText protocol and DevicePathFromText protocol.
+  The constructor function caches the pointer to DevicePathUtilites protocol.
 
   The constructor function locates these three protocols from protocol database.
   It will caches the pointer to local protocol instance if that operation fails
@@ -41,16 +40,12 @@ UefiDevicePathLibOptionalDevicePathProtocolConstructor (
   IN      EFI_SYSTEM_TABLE          *SystemTable
   )
 {
-  EFI_STATUS                        Status;
-
-  Status = gBS->LocateProtocol (
-                  &gEfiDevicePathUtilitiesProtocolGuid,
-                  NULL,
-                  (VOID**) &mDevicePathLibDevicePathUtilities
-                  );
-  ASSERT_EFI_ERROR (Status);
-  ASSERT (mDevicePathLibDevicePathUtilities != NULL);
-  return Status;
+  gBS->LocateProtocol (
+        &gEfiDevicePathUtilitiesProtocolGuid,
+        NULL,
+        (VOID**) &mDevicePathLibDevicePathUtilities
+        );
+  return EFI_SUCCESS;
 }
 
 /**
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
index e812e3e1d4..d194300bde 100644
--- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
@@ -5,7 +5,7 @@
 #  If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
 #  uses its internal conversion logic.
 #
-# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -79,6 +79,3 @@
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ## SOMETIMES_CONSUMES
 
-[Depex.common.DXE_DRIVER, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SAL_DRIVER, Depex.common.DXE_SMM_DRIVER]
-  gEfiDevicePathUtilitiesProtocolGuid
-
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
index 070f0add9f..7a7ebaaf1e 100644
--- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
@@ -5,14 +5,14 @@
 // If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
 // uses its internal conversion logic.
 //
-// Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 // **/
 
 
-#string STR_MODULE_ABSTRACT             #language en-US "Instance of Device Path Library based on Device Path Protocol."
+#string STR_MODULE_ABSTRACT             #language en-US "Instance of Device Path Library based on Device Path Protocol or Memory Allocation Library."
 
-#string STR_MODULE_DESCRIPTION          #language en-US "Instance of Device Path Library based on Device Path Protocol."
+#string STR_MODULE_DESCRIPTION          #language en-US "Instance of Device Path Library based on Device Path Protocol or Memory Allocation Library."
 
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build
  2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
  2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
@ 2019-12-18  2:10 ` Gao, Zhichao
  2019-12-18  8:28   ` Vitaly Cheptsov
  2019-12-18  8:26 ` [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Vitaly Cheptsov
  2019-12-20  5:49 ` Ni, Ray
  3 siblings, 1 reply; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-18  2:10 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Vitaly Cheptsov

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

Add the new instance lib for build.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Tested-by: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/MdePkg.dsc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 0aeafaaacc..9e9813dd27 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 # EFI/PI MdePkg Package
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
 #
 #    SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -99,6 +99,7 @@
   MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
   MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
   MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
+  MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
   MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
   MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   MdePkg/Library/UefiLib/UefiLib.inf
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
  2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
  2019-12-18  2:10 ` [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build Gao, Zhichao
@ 2019-12-18  8:26 ` Vitaly Cheptsov
  2019-12-19  1:51   ` [edk2-devel] " Gao, Zhichao
  2019-12-20  5:49 ` Ni, Ray
  3 siblings, 1 reply; 14+ messages in thread
From: Vitaly Cheptsov @ 2019-12-18  8:26 UTC (permalink / raw)
  To: Zhichao Gao, devel; +Cc: Michael D Kinney, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 2957 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib
  2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
@ 2019-12-18  8:27   ` Vitaly Cheptsov
  0 siblings, 0 replies; 14+ messages in thread
From: Vitaly Cheptsov @ 2019-12-18  8:27 UTC (permalink / raw)
  To: Zhichao Gao, devel; +Cc: Michael D Kinney, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 27232 bytes --]

Reviewed-by: Vitaly Cheptsov <vit9696@protonmail.com>

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
>
> UefiDevicePathLibOptionalDevicePathProtocol's implementation isn't
> fit its description. It should be implement as blow:
> Try to find the DevicePathProtocol, if found then use it to implement
> the interface. Else, use the local interface. It should not have the
> depex and ASSERT of gEfiDevicePathUtilitiesProtocolGuid when not find
> the DevicePathProtocol.
>
> Add a mandatory one to force using the DevicePathUtilities,
> DevicePathToText and DevicePathFromText protocol.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Tested-by: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>
> V2:
> Optional one should always return EFI_SUCCESS in its constructor.
> Change the description of optional one's uni file.
> Fix the copyright date of 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.
>
> ...DevicePathLibMandatoryDevicePathProtocol.c | 469 ++++++++++++++++++
> ...vicePathLibMandatoryDevicePathProtocol.inf | 86 ++++
> ...vicePathLibMandatoryDevicePathProtocol.uni | 18 +
> ...iDevicePathLibOptionalDevicePathProtocol.c | 21 +-
> ...evicePathLibOptionalDevicePathProtocol.inf | 5 +-
> ...evicePathLibOptionalDevicePathProtocol.uni | 6 +-
> 6 files changed, 585 insertions(+), 20 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
>
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c
> new file mode 100644
> index 0000000000..fa27110fd4
> --- /dev/null
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.c
> @@ -0,0 +1,469 @@
> +/** @file
> + Device Path services. The thing to remember is device paths are built out of
> + nodes. The device path is terminated by an end node that is length
> + sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is sizeof(EFI_DEVICE_PATH_PROTOCOL)
> + all over this file.
> +
> + The only place where multi-instance device paths are supported is in
> + environment varibles. Multi-instance device paths should never be placed
> + on a Handle.
> +
> + Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include "UefiDevicePathLib.h"
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_UTILITIES_PROTOCOL *mDevicePathLibDevicePathUtilities = NULL;
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *mDevicePathLibDevicePathToText = NULL;
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *mDevicePathLibDevicePathFromText = NULL;
> +
> +/**
> + The constructor function caches the pointer to DevicePathUtilites protocol,
> + DevicePathToText protocol and DevicePathFromText protocol.
> +
> + The constructor function locates these three protocols from protocol database.
> + It will caches the pointer to local protocol instance if that operation fails
> + and it will always return EFI_SUCCESS.
> +
> + @param ImageHandle The firmware allocated handle for the EFI image.
> + @param SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +UefiDevicePathLibMandatoryDevicePathProtocolConstructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = gBS->LocateProtocol (
> + &gEfiDevicePathUtilitiesProtocolGuid,
> + NULL,
> + (VOID**) &mDevicePathLibDevicePathUtilities
> + );
> + ASSERT_EFI_ERROR (Status);
> + ASSERT (mDevicePathLibDevicePathUtilities != NULL);
> +
> + Status = gBS->LocateProtocol (
> + &gEfiDevicePathToTextProtocolGuid,
> + NULL,
> + (VOID**) &mDevicePathLibDevicePathToText
> + );
> + ASSERT_EFI_ERROR (Status);
> + ASSERT (mDevicePathLibDevicePathToText != NULL);
> +
> + Status = gBS->LocateProtocol (
> + &gEfiDevicePathFromTextProtocolGuid,
> + NULL,
> + (VOID**) &mDevicePathLibDevicePathFromText
> + );
> + ASSERT_EFI_ERROR (Status);
> + ASSERT (mDevicePathLibDevicePathFromText != NULL);
> +
> + return Status;
> +}
> +
> +/**
> + Returns the size of a device path in bytes.
> +
> + This function returns the size, in bytes, of the device path data structure
> + specified by DevicePath including the end of device path node.
> + If DevicePath is NULL or invalid, then 0 is returned.
> +
> + @param DevicePath A pointer to a device path data structure.
> +
> + @retval 0 If DevicePath is NULL or invalid.
> + @retval Others The size of a device path in bytes.
> +
> +**/
> +UINTN
> +EFIAPI
> +GetDevicePathSize (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->GetDevicePathSize (DevicePath);
> + }
> +
> + ASSERT (FALSE);
> + return 0;
> +}
> +
> +/**
> + Creates a new copy of an existing device path.
> +
> + This function allocates space for a new copy of the device path specified by DevicePath.
> + If DevicePath is NULL, then NULL is returned. If the memory is successfully
> + allocated, then the contents of DevicePath are copied to the newly allocated
> + buffer, and a pointer to that buffer is returned. Otherwise, NULL is returned.
> + The memory for the new device path is allocated from EFI boot services memory.
> + It is the responsibility of the caller to free the memory allocated.
> +
> + @param DevicePath A pointer to a device path data structure.
> +
> + @retval NULL DevicePath is NULL or invalid.
> + @retval Others A pointer to the duplicated device path.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +DuplicateDevicePath (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->DuplicateDevicePath (DevicePath);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Creates a new device path by appending a second device path to a first device path.
> +
> + This function creates a new device path by appending a copy of SecondDevicePath
> + to a copy of FirstDevicePath in a newly allocated buffer. Only the end-of-device-path
> + device node from SecondDevicePath is retained. The newly created device path is
> + returned. If FirstDevicePath is NULL, then it is ignored, and a duplicate of
> + SecondDevicePath is returned. If SecondDevicePath is NULL, then it is ignored,
> + and a duplicate of FirstDevicePath is returned. If both FirstDevicePath and
> + SecondDevicePath are NULL, then a copy of an end-of-device-path is returned.
> +
> + If there is not enough memory for the newly allocated buffer, then NULL is returned.
> + The memory for the new device path is allocated from EFI boot services memory.
> + It is the responsibility of the caller to free the memory allocated.
> +
> + @param FirstDevicePath A pointer to a device path data structure.
> + @param SecondDevicePath A pointer to a device path data structure.
> +
> + @retval NULL If there is not enough memory for the newly allocated buffer.
> + @retval NULL If FirstDevicePath or SecondDevicePath is invalid.
> + @retval Others A pointer to the new device path if success.
> + Or a copy an end-of-device-path if both FirstDevicePath and SecondDevicePath are NULL.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +AppendDevicePath (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *FirstDevicePath, OPTIONAL
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *SecondDevicePath OPTIONAL
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->AppendDevicePath (FirstDevicePath, SecondDevicePath);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Creates a new path by appending the device node to the device path.
> +
> + This function creates a new device path by appending a copy of the device node
> + specified by DevicePathNode to a copy of the device path specified by DevicePath
> + in an allocated buffer. The end-of-device-path device node is moved after the
> + end of the appended device node.
> + If DevicePathNode is NULL then a copy of DevicePath is returned.
> + If DevicePath is NULL then a copy of DevicePathNode, followed by an end-of-device
> + path device node is returned.
> + If both DevicePathNode and DevicePath are NULL then a copy of an end-of-device-path
> + device node is returned.
> + If there is not enough memory to allocate space for the new device path, then
> + NULL is returned.
> + The memory is allocated from EFI boot services memory. It is the responsibility
> + of the caller to free the memory allocated.
> +
> + @param DevicePath A pointer to a device path data structure.
> + @param DevicePathNode A pointer to a single device path node.
> +
> + @retval NULL If there is not enough memory for the new device path.
> + @retval Others A pointer to the new device path if success.
> + A copy of DevicePathNode followed by an end-of-device-path node
> + if both FirstDevicePath and SecondDevicePath are NULL.
> + A copy of an end-of-device-path node if both FirstDevicePath
> + and SecondDevicePath are NULL.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +AppendDevicePathNode (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath, OPTIONAL
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePathNode OPTIONAL
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->AppendDeviceNode (DevicePath, DevicePathNode);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Creates a new device path by appending the specified device path instance to the specified device
> + path.
> +
> + This function creates a new device path by appending a copy of the device path
> + instance specified by DevicePathInstance to a copy of the device path specified
> + by DevicePath in a allocated buffer.
> + The end-of-device-path device node is moved after the end of the appended device
> + path instance and a new end-of-device-path-instance node is inserted between.
> + If DevicePath is NULL, then a copy if DevicePathInstance is returned.
> + If DevicePathInstance is NULL, then NULL is returned.
> + If DevicePath or DevicePathInstance is invalid, then NULL is returned.
> + If there is not enough memory to allocate space for the new device path, then
> + NULL is returned.
> + The memory is allocated from EFI boot services memory. It is the responsibility
> + of the caller to free the memory allocated.
> +
> + @param DevicePath A pointer to a device path data structure.
> + @param DevicePathInstance A pointer to a device path instance.
> +
> + @return A pointer to the new device path.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +AppendDevicePathInstance (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath, OPTIONAL
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePathInstance OPTIONAL
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->AppendDevicePathInstance (DevicePath, DevicePathInstance);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Creates a copy of the current device path instance and returns a pointer to the next device path
> + instance.
> +
> + This function creates a copy of the current device path instance. It also updates
> + DevicePath to point to the next device path instance in the device path (or NULL
> + if no more) and updates Size to hold the size of the device path instance copy.
> + If DevicePath is NULL, then NULL is returned.
> + If DevicePath points to a invalid device path, then NULL is returned.
> + If there is not enough memory to allocate space for the new device path, then
> + NULL is returned.
> + The memory is allocated from EFI boot services memory. It is the responsibility
> + of the caller to free the memory allocated.
> + If Size is NULL, then ASSERT().
> +
> + @param DevicePath On input, this holds the pointer to the current
> + device path instance. On output, this holds
> + the pointer to the next device path instance
> + or NULL if there are no more device path
> + instances in the device path pointer to a
> + device path data structure.
> + @param Size On output, this holds the size of the device
> + path instance, in bytes or zero, if DevicePath
> + is NULL.
> +
> + @return A pointer to the current device path instance.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +GetNextDevicePathInstance (
> + IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath,
> + OUT UINTN *Size
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->GetNextDevicePathInstance (DevicePath, Size);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Creates a device node.
> +
> + This function creates a new device node in a newly allocated buffer of size
> + NodeLength and initializes the device path node header with NodeType and NodeSubType.
> + The new device path node is returned.
> + If NodeLength is smaller than a device path header, then NULL is returned.
> + If there is not enough memory to allocate space for the new device path, then
> + NULL is returned.
> + The memory is allocated from EFI boot services memory. It is the responsibility
> + of the caller to free the memory allocated.
> +
> + @param NodeType The device node type for the new device node.
> + @param NodeSubType The device node sub-type for the new device node.
> + @param NodeLength The length of the new device node.
> +
> + @return The new device path.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +CreateDeviceNode (
> + IN UINT8 NodeType,
> + IN UINT8 NodeSubType,
> + IN UINT16 NodeLength
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->CreateDeviceNode (NodeType, NodeSubType, NodeLength);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Determines if a device path is single or multi-instance.
> +
> + This function returns TRUE if the device path specified by DevicePath is
> + multi-instance.
> + Otherwise, FALSE is returned.
> + If DevicePath is NULL or invalid, then FALSE is returned.
> +
> + @param DevicePath A pointer to a device path data structure.
> +
> + @retval TRUE DevicePath is multi-instance.
> + @retval FALSE DevicePath is not multi-instance, or DevicePath
> + is NULL or invalid.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +IsDevicePathMultiInstance (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath
> + )
> +{
> + if (mDevicePathLibDevicePathUtilities != NULL) {
> + return mDevicePathLibDevicePathUtilities->IsDevicePathMultiInstance (DevicePath);
> + }
> +
> + ASSERT (FALSE);
> + return FALSE;
> +}
> +
> +/**
> + Converts a device node to its string representation.
> +
> + @param DeviceNode A Pointer to the device node to be converted.
> + @param DisplayOnly If DisplayOnly is TRUE, then the shorter text representation
> + of the display node is used, where applicable. If DisplayOnly
> + is FALSE, then the longer text representation of the display node
> + is used.
> + @param AllowShortcuts If AllowShortcuts is TRUE, then the shortcut forms of text
> + representation for a device node can be used, where applicable.
> +
> + @return A pointer to the allocated text representation of the device node or NULL if DeviceNode
> + is NULL or there was insufficient memory.
> +
> +**/
> +CHAR16 *
> +EFIAPI
> +ConvertDeviceNodeToText (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DeviceNode,
> + IN BOOLEAN DisplayOnly,
> + IN BOOLEAN AllowShortcuts
> + )
> +{
> + if (mDevicePathLibDevicePathToText != NULL) {
> + return mDevicePathLibDevicePathToText->ConvertDeviceNodeToText (DeviceNode, DisplayOnly, AllowShortcuts);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Converts a device path to its text representation.
> +
> + @param DevicePath A Pointer to the device to be converted.
> + @param DisplayOnly If DisplayOnly is TRUE, then the shorter text representation
> + of the display node is used, where applicable. If DisplayOnly
> + is FALSE, then the longer text representation of the display node
> + is used.
> + @param AllowShortcuts If AllowShortcuts is TRUE, then the shortcut forms of text
> + representation for a device node can be used, where applicable.
> +
> + @return A pointer to the allocated text representation of the device path or
> + NULL if DeviceNode is NULL or there was insufficient memory.
> +
> +**/
> +CHAR16 *
> +EFIAPI
> +ConvertDevicePathToText (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath,
> + IN BOOLEAN DisplayOnly,
> + IN BOOLEAN AllowShortcuts
> + )
> +{
> + if (mDevicePathLibDevicePathToText != NULL) {
> + return mDevicePathLibDevicePathToText->ConvertDevicePathToText (DevicePath, DisplayOnly, AllowShortcuts);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Convert text to the binary representation of a device node.
> +
> + @param TextDeviceNode TextDeviceNode points to the text representation of a device
> + node. Conversion starts with the first character and continues
> + until the first non-device node character.
> +
> + @return A pointer to the EFI device node or NULL if TextDeviceNode is NULL or there was
> + insufficient memory or text unsupported.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +ConvertTextToDeviceNode (
> + IN CONST CHAR16 *TextDeviceNode
> + )
> +{
> + if (mDevicePathLibDevicePathFromText != NULL) {
> + return mDevicePathLibDevicePathFromText->ConvertTextToDeviceNode (TextDeviceNode);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> +/**
> + Convert text to the binary representation of a device path.
> +
> +
> + @param TextDevicePath TextDevicePath points to the text representation of a device
> + path. Conversion starts with the first character and continues
> + until the first non-device node character.
> +
> + @return A pointer to the allocated device path or NULL if TextDeviceNode is NULL or
> + there was insufficient memory.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +EFIAPI
> +ConvertTextToDevicePath (
> + IN CONST CHAR16 *TextDevicePath
> + )
> +{
> + if (mDevicePathLibDevicePathFromText != NULL) {
> + return mDevicePathLibDevicePathFromText->ConvertTextToDevicePath (TextDevicePath);
> + }
> +
> + ASSERT (FALSE);
> + return NULL;
> +}
> +
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
> new file mode 100644
> index 0000000000..eb545d4601
> --- /dev/null
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
> @@ -0,0 +1,86 @@
> +## @file
> +# Instance of Device Path Library based on Device Path Protocol.
> +#
> +# Device Path Library that layers on top of the UEFI 2.0 Device Path Protocol.
> +# If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
> +# uses its internal conversion logic.
> +#
> +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = UefiDevicePathLibMandatoryDevicePathProtocol
> + MODULE_UNI_FILE = UefiDevicePathLibMandatoryDevicePathProtocol.uni
> + FILE_GUID = 5A8389AF-DAE4-407E-8FD5-D026948BC579
> + MODULE_TYPE = UEFI_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = DevicePathLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE
> +
> + CONSTRUCTOR = UefiDevicePathLibMandatoryDevicePathProtocolConstructor
> +
> +#
> +# VALID_ARCHITECTURES = IA32 X64 EBC
> +#
> +
> +[Sources]
> + DevicePathUtilities.c
> + DevicePathToText.c
> + DevicePathFromText.c
> + UefiDevicePathLibMandatoryDevicePathProtocol.c
> + UefiDevicePathLib.h
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> +
> +
> +[LibraryClasses]
> + BaseLib
> + UefiBootServicesTableLib
> + MemoryAllocationLib
> + DebugLib
> + BaseMemoryLib
> + PcdLib
> + PrintLib
> +
> +[Guids]
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiVTUTF8Guid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiVT100Guid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiVT100PlusGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiPcAnsiGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiUartDevicePathGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiSasDevicePathGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiVirtualDiskGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiVirtualCdGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiPersistentVirtualDiskGuid
> + ## SOMETIMES_CONSUMES ## GUID
> + gEfiPersistentVirtualCdGuid
> +
> +[Protocols]
> + gEfiDevicePathProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiDevicePathUtilitiesProtocolGuid ## CONSUMES
> + gEfiDevicePathToTextProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiDevicePathFromTextProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiDebugPortProtocolGuid ## UNDEFINED
> +
> +[Pcd]
> + gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ## SOMETIMES_CONSUMES
> +
> +[Depex.common.DXE_DRIVER, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SAL_DRIVER, Depex.common.DXE_SMM_DRIVER]
> + gEfiDevicePathUtilitiesProtocolGuid AND
> + gEfiDevicePathToTextProtocolGuid AND
> + gEfiDevicePathFromTextProtocolGuid
> +
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni
> new file mode 100644
> index 0000000000..1b8a556698
> --- /dev/null
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.uni
> @@ -0,0 +1,18 @@
> +// /** @file
> +// Instance of Device Path Library based on Device Path Protocol.
> +//
> +// Device Path Library that layers on top of the UEFI 2.0 Device Path Protocol.
> +// If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
> +// uses its internal conversion logic.
> +//
> +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT #language en-US "Instance of Device Path Library based on Device Path Protocol."
> +
> +#string STR_MODULE_DESCRIPTION #language en-US "Instance of Device Path Library based on Device Path Protocol."
> +
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
> index 106ff245cc..ddef75da18 100644
> --- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.c
> @@ -8,7 +8,7 @@
> environment varibles. Multi-instance device paths should never be placed
> on a Handle.
>
> - Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -21,8 +21,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *mDevicePathLib
> GLOBAL_REMOVE_IF_UNREFERENCED EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *mDevicePathLibDevicePathFromText = NULL;
>
> /**
> - The constructor function caches the pointer to DevicePathUtilites protocol,
> - DevicePathToText protocol and DevicePathFromText protocol.
> + The constructor function caches the pointer to DevicePathUtilites protocol.
>
> The constructor function locates these three protocols from protocol database.
> It will caches the pointer to local protocol instance if that operation fails
> @@ -41,16 +40,12 @@ UefiDevicePathLibOptionalDevicePathProtocolConstructor (
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> - EFI_STATUS Status;
> -
> - Status = gBS->LocateProtocol (
> - &gEfiDevicePathUtilitiesProtocolGuid,
> - NULL,
> - (VOID**) &mDevicePathLibDevicePathUtilities
> - );
> - ASSERT_EFI_ERROR (Status);
> - ASSERT (mDevicePathLibDevicePathUtilities != NULL);
> - return Status;
> + gBS->LocateProtocol (
> + &gEfiDevicePathUtilitiesProtocolGuid,
> + NULL,
> + (VOID**) &mDevicePathLibDevicePathUtilities
> + );
> + return EFI_SUCCESS;
> }
>
> /**
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
> index e812e3e1d4..d194300bde 100644
> --- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
> @@ -5,7 +5,7 @@
> # If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
> # uses its internal conversion logic.
> #
> -# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -79,6 +79,3 @@
> [Pcd]
> gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ## SOMETIMES_CONSUMES
>
> -[Depex.common.DXE_DRIVER, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SAL_DRIVER, Depex.common.DXE_SMM_DRIVER]
> - gEfiDevicePathUtilitiesProtocolGuid
> -
> diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
> index 070f0add9f..7a7ebaaf1e 100644
> --- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
> +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.uni
> @@ -5,14 +5,14 @@
> // If the DevicePathFromText/DevicePathToText protocol doesn't exist, the library
> // uses its internal conversion logic.
> //
> -// Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> +// Copyright (c) 2013 - 2019, Intel Corporation. All rights reserved.<BR>
> //
> // SPDX-License-Identifier: BSD-2-Clause-Patent
> //
> // **/
>
> -#string STR_MODULE_ABSTRACT #language en-US "Instance of Device Path Library based on Device Path Protocol."
> +#string STR_MODULE_ABSTRACT #language en-US "Instance of Device Path Library based on Device Path Protocol or Memory Allocation Library."
>
> -#string STR_MODULE_DESCRIPTION #language en-US "Instance of Device Path Library based on Device Path Protocol."
> +#string STR_MODULE_DESCRIPTION #language en-US "Instance of Device Path Library based on Device Path Protocol or Memory Allocation Library."
>
> --
> 2.21.0.windows.1

[-- Attachment #2: Type: text/html, Size: 31302 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build
  2019-12-18  2:10 ` [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build Gao, Zhichao
@ 2019-12-18  8:28   ` Vitaly Cheptsov
  0 siblings, 0 replies; 14+ messages in thread
From: Vitaly Cheptsov @ 2019-12-18  8:28 UTC (permalink / raw)
  To: Zhichao Gao, devel; +Cc: Michael D Kinney, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

Reviewed-by: Vitaly Cheptsov <vit9696@protonmail.com>

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
>
> Add the new instance lib for build.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
> Tested-by: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
> MdePkg/MdePkg.dsc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index 0aeafaaacc..9e9813dd27 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -1,7 +1,7 @@
> ## @file
> # EFI/PI MdePkg Package
> #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
> # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -99,6 +99,7 @@
> MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
> + MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePathProtocol.inf
> MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
> MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
> MdePkg/Library/UefiLib/UefiLib.inf
> --
> 2.21.0.windows.1

[-- Attachment #2: Type: text/html, Size: 2503 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-18  8:26 ` [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Vitaly Cheptsov
@ 2019-12-19  1:51   ` Gao, Zhichao
  0 siblings, 0 replies; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-19  1:51 UTC (permalink / raw)
  To: devel@edk2.groups.io, 'vit9696@protonmail.com'
  Cc: Kinney, Michael D, Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 3175 bytes --]

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<mailto: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<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com<mailto: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




[-- Attachment #2: Type: text/html, Size: 7312 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
                   ` (2 preceding siblings ...)
  2019-12-18  8:26 ` [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Vitaly Cheptsov
@ 2019-12-20  5:49 ` Ni, Ray
  2019-12-20  6:03   ` Gao, Zhichao
  3 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2019-12-20  5:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Zhichao
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

Zhichao,
\MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains the version that hard-depends on the protocol.
I don't think you need to add another version.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> Zhichao
> Sent: Wednesday, December 18, 2019 10:11 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> the lib instances
> 
> 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/UefiDevicePathLibMandatoryDevicePat
> hProtocol.c
>  create mode 100644
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> hProtocol.inf
>  create mode 100644
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> hProtocol.uni
> 
> --
> 2.21.0.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-20  5:49 ` Ni, Ray
@ 2019-12-20  6:03   ` Gao, Zhichao
  2019-12-20  6:19     ` Ni, Ray
  0 siblings, 1 reply; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-20  6:03 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

Ray,

I knew there is one in MdePkg. But it has duplicate code with UefiDevicePathLib. That is why I add the Mandatory one.
And it is recommended to use the one in UefiDevicePathLib path.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, December 20, 2019 1:50 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> the lib instances
> 
> Zhichao,
> \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains the version
> that hard-depends on the protocol.
> I don't think you need to add another version.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> > Zhichao
> > Sent: Wednesday, December 18, 2019 10:11 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate the lib instances
> >
> > 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/UefiDevicePathLibMandatoryDevicePat
> > hProtocol.c
> >  create mode 100644
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > hProtocol.inf
> >  create mode 100644
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > hProtocol.uni
> >
> > --
> > 2.21.0.windows.1
> >
> >
> > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-20  6:03   ` Gao, Zhichao
@ 2019-12-20  6:19     ` Ni, Ray
  2019-12-20  6:40       ` Gao, Zhichao
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2019-12-20  6:19 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

Removing code duplication is great.

But your patch introduces more code duplication: the mandatory one in UefiDevicePathLib
directory and the other one in UefiDevicePathLibDevicePathProtocol directory.

Do you have a plan to remove the one in UefiDevicePathLibDevicePathProtocol directory?
Have you evaluated the impact to consumers of removing that one?

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Friday, December 20, 2019 2:03 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> Separate the lib instances
> 
> Ray,
> 
> I knew there is one in MdePkg. But it has duplicate code with
> UefiDevicePathLib. That is why I add the Mandatory one.
> And it is recommended to use the one in UefiDevicePathLib path.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Friday, December 20, 2019 1:50 PM
> > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> Separate
> > the lib instances
> >
> > Zhichao,
> > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains the
> version
> > that hard-depends on the protocol.
> > I don't think you need to add another version.
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> > > Zhichao
> > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > To: devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > Separate the lib instances
> > >
> > > 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/UefiDevicePathLibMandatoryDevicePat
> > > hProtocol.c
> > >  create mode 100644
> > >
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > hProtocol.inf
> > >  create mode 100644
> > >
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > hProtocol.uni
> > >
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-20  6:19     ` Ni, Ray
@ 2019-12-20  6:40       ` Gao, Zhichao
  2019-12-20  7:16         ` Ni, Ray
  0 siblings, 1 reply; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-20  6:40 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

For open source, it would impact fsp2 package, ovmf package and some open platform packages. Not sure for others.
I didn't plan the removal of UefiDevicePathLibDevicePathProtocol yet.

Thanks,
Zhichao
> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, December 20, 2019 2:20 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> the lib instances
> 
> Removing code duplication is great.
> 
> But your patch introduces more code duplication: the mandatory one in
> UefiDevicePathLib directory and the other one in
> UefiDevicePathLibDevicePathProtocol directory.
> 
> Do you have a plan to remove the one in UefiDevicePathLibDevicePathProtocol
> directory?
> Have you evaluated the impact to consumers of removing that one?
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Friday, December 20, 2019 2:03 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate the lib instances
> >
> > Ray,
> >
> > I knew there is one in MdePkg. But it has duplicate code with
> > UefiDevicePathLib. That is why I add the Mandatory one.
> > And it is recommended to use the one in UefiDevicePathLib path.
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Friday, December 20, 2019 1:50 PM
> > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate
> > > the lib instances
> > >
> > > Zhichao,
> > > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains the
> > version
> > > that hard-depends on the protocol.
> > > I don't think you need to add another version.
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Gao, Zhichao
> > > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > Separate the lib instances
> > > >
> > > > 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/UefiDevicePathLibMandatoryDevicePat
> > > > hProtocol.c
> > > >  create mode 100644
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > hProtocol.inf
> > > >  create mode 100644
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > hProtocol.uni
> > > >
> > > > --
> > > > 2.21.0.windows.1
> > > >
> > > >
> > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-20  6:40       ` Gao, Zhichao
@ 2019-12-20  7:16         ` Ni, Ray
  2019-12-23  0:43           ` Gao, Zhichao
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2019-12-20  7:16 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

Zhichao,
I prefer to have one patch serial which include:
1. Adds new mandatory instance
2. Update consumers to use the new instance
2. Removes the old mandatory instance

Otherwise, adding a new mandatory instance introduces more
code duplication IMO.

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Friday, December 20, 2019 2:41 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> Separate the lib instances
> 
> For open source, it would impact fsp2 package, ovmf package and some
> open platform packages. Not sure for others.
> I didn't plan the removal of UefiDevicePathLibDevicePathProtocol yet.
> 
> Thanks,
> Zhichao
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Friday, December 20, 2019 2:20 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> Separate
> > the lib instances
> >
> > Removing code duplication is great.
> >
> > But your patch introduces more code duplication: the mandatory one in
> > UefiDevicePathLib directory and the other one in
> > UefiDevicePathLibDevicePathProtocol directory.
> >
> > Do you have a plan to remove the one in
> UefiDevicePathLibDevicePathProtocol
> > directory?
> > Have you evaluated the impact to consumers of removing that one?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > Sent: Friday, December 20, 2019 2:03 PM
> > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > Separate the lib instances
> > >
> > > Ray,
> > >
> > > I knew there is one in MdePkg. But it has duplicate code with
> > > UefiDevicePathLib. That is why I add the Mandatory one.
> > > And it is recommended to use the one in UefiDevicePathLib path.
> > >
> > > Thanks,
> > > Zhichao
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray
> > > > Sent: Friday, December 20, 2019 1:50 PM
> > > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > Separate
> > > > the lib instances
> > > >
> > > > Zhichao,
> > > > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains the
> > > version
> > > > that hard-depends on the protocol.
> > > > I don't think you need to add another version.
> > > >
> > > > Thanks,
> > > > Ray
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Gao, Zhichao
> > > > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > Separate the lib instances
> > > > >
> > > > > 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/UefiDevicePathLibMandatoryDevicePat
> > > > > hProtocol.c
> > > > >  create mode 100644
> > > > >
> > >
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > hProtocol.inf
> > > > >  create mode 100644
> > > > >
> > >
> MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > hProtocol.uni
> > > > >
> > > > > --
> > > > > 2.21.0.windows.1
> > > > >
> > > > >
> > > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-20  7:16         ` Ni, Ray
@ 2019-12-23  0:43           ` Gao, Zhichao
  2019-12-23  8:01             ` Liming Gao
  0 siblings, 1 reply; 14+ messages in thread
From: Gao, Zhichao @ 2019-12-23  0:43 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Vitaly Cheptsov

Ray,

Your suggestion is good for open source, but unfriendly to the close source platforms which consume this lib.

Hi Mike/Liming/Ray/Others,

Do we have a progress to retire lib/API/others in the open source, like below?
1. Announce that there is something going to retire.
2. Suggestion to replace the retired function in open source. Or the justification of the retirement.
3. Collect the feedback especially the disagreement.
4. Discuss and make the decision.
5. Reject the retirement. Or announce the retire date to let consumers to change their platform codes.
6. Retire the function.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, December 20, 2019 3:16 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> the lib instances
> 
> Zhichao,
> I prefer to have one patch serial which include:
> 1. Adds new mandatory instance
> 2. Update consumers to use the new instance 2. Removes the old mandatory
> instance
> 
> Otherwise, adding a new mandatory instance introduces more code duplication
> IMO.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Friday, December 20, 2019 2:41 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate the lib instances
> >
> > For open source, it would impact fsp2 package, ovmf package and some
> > open platform packages. Not sure for others.
> > I didn't plan the removal of UefiDevicePathLibDevicePathProtocol yet.
> >
> > Thanks,
> > Zhichao
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Friday, December 20, 2019 2:20 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate
> > > the lib instances
> > >
> > > Removing code duplication is great.
> > >
> > > But your patch introduces more code duplication: the mandatory one
> > > in UefiDevicePathLib directory and the other one in
> > > UefiDevicePathLibDevicePathProtocol directory.
> > >
> > > Do you have a plan to remove the one in
> > UefiDevicePathLibDevicePathProtocol
> > > directory?
> > > Have you evaluated the impact to consumers of removing that one?
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -----Original Message-----
> > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > Sent: Friday, December 20, 2019 2:03 PM
> > > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > Separate the lib instances
> > > >
> > > > Ray,
> > > >
> > > > I knew there is one in MdePkg. But it has duplicate code with
> > > > UefiDevicePathLib. That is why I add the Mandatory one.
> > > > And it is recommended to use the one in UefiDevicePathLib path.
> > > >
> > > > Thanks,
> > > > Zhichao
> > > >
> > > > > -----Original Message-----
> > > > > From: Ni, Ray
> > > > > Sent: Friday, December 20, 2019 1:50 PM
> > > > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > Separate
> > > > > the lib instances
> > > > >
> > > > > Zhichao,
> > > > > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains
> > > > > the
> > > > version
> > > > > that hard-depends on the protocol.
> > > > > I don't think you need to add another version.
> > > > >
> > > > > Thanks,
> > > > > Ray
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > Gao, Zhichao
> > > > > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> > > > > > Liming <liming.gao@intel.com>; Vitaly Cheptsov
> > > > > > <vit9696@protonmail.com>
> > > > > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > > Separate the lib instances
> > > > > >
> > > > > > 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/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.c
> > > > > >  create mode 100644
> > > > > >
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.inf
> > > > > >  create mode 100644
> > > > > >
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.uni
> > > > > >
> > > > > > --
> > > > > > 2.21.0.windows.1
> > > > > >
> > > > > >
> > > > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
  2019-12-23  0:43           ` Gao, Zhichao
@ 2019-12-23  8:01             ` Liming Gao
  0 siblings, 0 replies; 14+ messages in thread
From: Liming Gao @ 2019-12-23  8:01 UTC (permalink / raw)
  To: Gao, Zhichao, Ni, Ray, devel@edk2.groups.io
  Cc: Kinney, Michael D, Vitaly Cheptsov

Zhichao:
  I think the first step is to send RFC on the proposal. If no one rejects RFC, then work out the detail changes. 

Thanks
Liming
> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Monday, December 23, 2019 8:44 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
> 
> Ray,
> 
> Your suggestion is good for open source, but unfriendly to the close source platforms which consume this lib.
> 
> Hi Mike/Liming/Ray/Others,
> 
> Do we have a progress to retire lib/API/others in the open source, like below?
> 1. Announce that there is something going to retire.
> 2. Suggestion to replace the retired function in open source. Or the justification of the retirement.
> 3. Collect the feedback especially the disagreement.
> 4. Discuss and make the decision.
> 5. Reject the retirement. Or announce the retire date to let consumers to change their platform codes.
> 6. Retire the function.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Friday, December 20, 2019 3:16 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> > the lib instances
> >
> > Zhichao,
> > I prefer to have one patch serial which include:
> > 1. Adds new mandatory instance
> > 2. Update consumers to use the new instance 2. Removes the old mandatory
> > instance
> >
> > Otherwise, adding a new mandatory instance introduces more code duplication
> > IMO.
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > Sent: Friday, December 20, 2019 2:41 PM
> > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > Separate the lib instances
> > >
> > > For open source, it would impact fsp2 package, ovmf package and some
> > > open platform packages. Not sure for others.
> > > I didn't plan the removal of UefiDevicePathLibDevicePathProtocol yet.
> > >
> > > Thanks,
> > > Zhichao
> > > > -----Original Message-----
> > > > From: Ni, Ray
> > > > Sent: Friday, December 20, 2019 2:20 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>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > Separate
> > > > the lib instances
> > > >
> > > > Removing code duplication is great.
> > > >
> > > > But your patch introduces more code duplication: the mandatory one
> > > > in UefiDevicePathLib directory and the other one in
> > > > UefiDevicePathLibDevicePathProtocol directory.
> > > >
> > > > Do you have a plan to remove the one in
> > > UefiDevicePathLibDevicePathProtocol
> > > > directory?
> > > > Have you evaluated the impact to consumers of removing that one?
> > > >
> > > > Thanks,
> > > > Ray
> > > >
> > > > > -----Original Message-----
> > > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > > Sent: Friday, December 20, 2019 2:03 PM
> > > > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > Separate the lib instances
> > > > >
> > > > > Ray,
> > > > >
> > > > > I knew there is one in MdePkg. But it has duplicate code with
> > > > > UefiDevicePathLib. That is why I add the Mandatory one.
> > > > > And it is recommended to use the one in UefiDevicePathLib path.
> > > > >
> > > > > Thanks,
> > > > > Zhichao
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ni, Ray
> > > > > > Sent: Friday, December 20, 2019 1:50 PM
> > > > > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > Separate
> > > > > > the lib instances
> > > > > >
> > > > > > Zhichao,
> > > > > > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains
> > > > > > the
> > > > > version
> > > > > > that hard-depends on the protocol.
> > > > > > I don't think you need to add another version.
> > > > > >
> > > > > > Thanks,
> > > > > > Ray
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > > Gao, Zhichao
> > > > > > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > > > > > To: devel@edk2.groups.io
> > > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> > > > > > > Liming <liming.gao@intel.com>; Vitaly Cheptsov
> > > > > > > <vit9696@protonmail.com>
> > > > > > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > > > Separate the lib instances
> > > > > > >
> > > > > > > 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/UefiDevicePathLibMandatoryDevicePat
> > > > > > > hProtocol.c
> > > > > > >  create mode 100644
> > > > > > >
> > > > >
> > > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > > hProtocol.inf
> > > > > > >  create mode 100644
> > > > > > >
> > > > >
> > > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > > hProtocol.uni
> > > > > > >
> > > > > > > --
> > > > > > > 2.21.0.windows.1
> > > > > > >
> > > > > > >
> > > > > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-12-23  8:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
2019-12-18  8:27   ` Vitaly Cheptsov
2019-12-18  2:10 ` [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build Gao, Zhichao
2019-12-18  8:28   ` Vitaly Cheptsov
2019-12-18  8:26 ` [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Vitaly Cheptsov
2019-12-19  1:51   ` [edk2-devel] " Gao, Zhichao
2019-12-20  5:49 ` Ni, Ray
2019-12-20  6:03   ` Gao, Zhichao
2019-12-20  6:19     ` Ni, Ray
2019-12-20  6:40       ` Gao, Zhichao
2019-12-20  7:16         ` Ni, Ray
2019-12-23  0:43           ` Gao, Zhichao
2019-12-23  8:01             ` Liming Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox