Replying as per Liming's request for this to be merged into edk2-stable202002.
On Mon, Feb 10, 2020 at 14:12, vit9696 <vit9...@protonmail.com> wrote: > Hello, > > It has been quite some time since we submitted the patch with so far no > negative response. As I mentioned previously, my team will strongly benefit > from its landing in EDK II mainline. Since it does not add any regressions > and can be viewed as a feature implementation for the rest of EDK II users, I > request this to be merged upstream in edk2-stable202002. > > Best wishes, > Vitaly > >> 27 янв. 2020 г., в 12:47, vit9696 <vit9...@protonmail.com> написал(а): >> >> >> Hi Mike, >> >> Any progress with this? We would really benefit from this landing in the >> next stable release. >> >> Best, >> Vitaly >> >>> 8 янв. 2020 г., в 19:35, Kinney, Michael D <michael.d.kin...@intel.com> >>> написал(а): >>> >>> >>> Hi Vitaly, >>> >>> Thanks for the additional background. I would like >>> a couple extra day to review the PCD name and the places >>> the PCD might potentially be used. >>> >>> If we find other APIs where ASSERT() behavior is only >>> valuable during dev/debug to quickly identify misuse >>> with trusted data and the API provides predicable >>> return behavior when ASSERT() is disabled, then I would >>> like to have a pattern we can potentially apply to all >>> these APIs across all packages. >>> >>> Thanks, >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On >>>> Behalf Of Vitaly Cheptsov via Groups.Io >>>> Sent: Monday, January 6, 2020 10:44 AM >>>> To: Kinney, Michael D <michael.d.kin...@intel.com> >>>> Cc: devel@edk2.groups.io >>>> Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to >>>> disable safe string constraint assertions >>>> >>>> Hi Mike, >>>> >>>> Yes, the primary use case is for UEFI Applications. We >>>> do not want to disable ASSERT’s completely, as >>>> assertions that make sense, i.e. the ones signalising >>>> about interface misuse, are helpful for debugging. >>>> >>>> I have already explained in the BZ that basically all >>>> safe string constraint assertions make no sense for >>>> handling untrusted data. We find this use case very >>>> logical, as these functions behave properly with >>>> assertions disabled and cover all these error >>>> conditions by the return statuses. In such situation is >>>> not useful for these functions to assert, as we end up >>>> inefficiently reimplementing the logic. I would have >>>> liked the approach of discussing the interfaces >>>> individually, but I struggle to find any that makes >>>> sense from this point of view. >>>> >>>> AsciiStrToGuid will ASSERT when the length of the >>>> passed string is odd. Functions that cannot, ahem, >>>> parse, for us are pretty much useless. >>>> AsciiStrCatS will ASSERT when the appended string does >>>> not fit the buffer. For us this logic makes this >>>> function pretty much equivalent to deprecated and thus >>>> unavailable AsciiStrCat, except it is also slower. >>>> >>>> My original suggestion was to remove the assertions >>>> entirely, but several people here said that they use >>>> them to verify usage errors when handling trusted data. >>>> This makes good sense to me, so we suggest to support >>>> both cases by introducing a PCD in this patch. >>>> >>>> Best wishes, >>>> Vitaly >>>> >>>>> 6 янв. 2020 г., в 21:28, Kinney, Michael D >>>> <michael.d.kin...@intel.com> написал(а): >>>>> >>>>> >>>>> Hi Vitaly, >>>>> >>>>> Is the use case for UEFI Applications? >>>>> >>>>> There is a different mechanism to disable all >>>> ASSERT() >>>>> statements within a UEFI Application. >>>>> >>>>> If a component is consuming data from an untrusted >>>> source, >>>>> then that component is required to verify the >>>> untrusted >>>>> data before passing it to a function that clearly >>>> documents >>>>> is input requirements. If this approach is followed, >>>> then >>>>> the BaseLib functions can be used "as is" as long as >>>> the >>>>> ASSERT() conditions are verified before calling. >>>>> >>>>> If there are some APIs that currently document their >>>> ASSERT() >>>>> behavior and we think that ASSERT() behavior is >>>> incorrect and >>>>> should be handled by an existing error return value, >>>> then we >>>>> should discuss each of those APIs individually. >>>>> >>>>> Mike >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On >>>>>> Behalf Of Vitaly Cheptsov via Groups.Io >>>>>> Sent: Friday, January 3, 2020 9:13 AM >>>>>> To: devel@edk2.groups.io >>>>>> Subject: [edk2-devel] [PATCH v3 0/1] Add PCD to >>>> disable >>>>>> safe string constraint assertions >>>>>> >>>>>> REF: >>>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=2054 >>>>>> >>>>>> Requesting for merge in edk2-stable202002. >>>>>> >>>>>> Changes since V1: >>>>>> - Enable assertions by default to preserve the >>>> original >>>>>> behaviour >>>>>> - Fix bugzilla reference link >>>>>> - Update documentation in BaseLib.h >>>>>> >>>>>> Vitaly Cheptsov (1): >>>>>> MdePkg: Add PCD to disable safe string constraint >>>>>> assertions >>>>>> >>>>>> MdePkg/MdePkg.dec | 6 ++ >>>>>> MdePkg/Library/BaseLib/BaseLib.inf | 11 +-- >>>>>> MdePkg/Include/Library/BaseLib.h | 74 >>>>>> +++++++++++++------- >>>>>> MdePkg/Library/BaseLib/SafeString.c | 4 +- >>>>>> MdePkg/MdePkg.uni | 6 ++ >>>>>> 5 files changed, 71 insertions(+), 30 deletions(-) >>>>>> >>>>>> -- >>>>>> 2.21.0 (Apple Git-122.2) >>>>>> >>>>>> >>>>>> -=-=-=-=-=-= >>>>>> Groups.io Links: You receive all messages sent to >>>> this >>>>>> group. >>>>>> >>>>>> View/Reply Online (#52837): >>>>>> https://edk2.groups.io/g/devel/message/52837 >>>>>> Mute This Topic: >>>> https://groups.io/mt/69401948/1643496 >>>>>> Group Owner: devel+ow...@edk2.groups.io >>>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >>>>>> [michael.d.kin...@intel.com] >>>>>> -=-=-=-=-=-= >>>>> >>>> >>>> >>>> >>> >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54438): https://edk2.groups.io/g/devel/message/54438 Mute This Topic: https://groups.io/mt/69401948/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
