...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
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52315): https://edk2.groups.io/g/devel/message/52315 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52334): https://edk2.groups.io/g/devel/message/52334 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52392): https://edk2.groups.io/g/devel/message/52392 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52441): https://edk2.groups.io/g/devel/message/52441 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52442): https://edk2.groups.io/g/devel/message/52442 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52446): https://edk2.groups.io/g/devel/message/52446 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52449): https://edk2.groups.io/g/devel/message/52449 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52450): https://edk2.groups.io/g/devel/message/52450 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52478): https://edk2.groups.io/g/devel/message/52478 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52501): https://edk2.groups.io/g/devel/message/52501 Mute This Topic: https://groups.io/mt/68779976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.