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 написал(а): > > > 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 On >> Behalf Of Vitaly Cheptsov via Groups.Io >> Sent: Monday, January 6, 2020 10:44 AM >> To: Kinney, Michael D >> 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 >> написал(а): >>> >>> >>> 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 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+owner@edk2.groups.io >>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >>>> [michael.d.kinney@intel.com] >>>> -=-=-=-=-=-= >>> >> >> >> >