From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.45]) by mx.groups.io with SMTP id smtpd.web12.1739.1589309874283764881 for ; Tue, 12 May 2020 11:57:54 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: ispras.ru, ip: 83.149.199.45, mailfrom: cheptsov@ispras.ru) Received: from [127.0.0.1] (unknown [77.232.9.83]) by mail.ispras.ru (Postfix) with ESMTPSA id 66038CD466; Tue, 12 May 2020 21:57:52 +0300 (MSK) From: "Vitaly Cheptsov" Message-Id: <09ED0896-A19B-4710-A92A-46B678BF94D4@ispras.ru> Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Subject: Re: [edk2-devel] [PATCH V4 00/27] Disabling safe string constraint assertions Date: Tue, 12 May 2020 21:57:50 +0300 In-Reply-To: Cc: "devel@edk2.groups.io" , Andrew Fish , =?utf-8?Q?Marvin_H=C3=A4user?= , "Gao, Liming" , "Gao, Zhichao" To: "Kinney, Michael D" , "lersek@redhat.com" References: <20200511154121.3878-1-cheptsov@ispras.ru> <44ac1ca1-953a-21a2-0c9e-c83aca153b0b@redhat.com> X-Mailer: Apple Mail (2.3608.80.23.2.2) X-Groupsio-MsgNum: 59356 Content-Type: multipart/signed; boundary="Apple-Mail=_B8EF74E7-4C13-42B5-A12B-6102CAE97DAF"; protocol="application/pgp-signature"; micalg=pgp-sha512 --Apple-Mail=_B8EF74E7-4C13-42B5-A12B-6102CAE97DAF Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Mike, I see what you mean here, but I believe it is absolutely fine to implement= DebugCommonLib.h interface within DebugLib without depending on DebugCommo= nLib if one absolutely desires it (thought I do not think it is a good idea= ). In this case, perhaps we can avoid adding DebugCommonLib.h header at all a= nd leave it all in DebugLib.h? So that DebugCommonLib library is entirely private thing to DebugLib, whic= h can either use it or not. Best wishes, Vitaly > 12 =D0=BC=D0=B0=D1=8F 2020 =D0=B3., =D0=B2 21:18, Kinney, Michael D =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0)= : >=20 > Laszlo, >=20 > A library implementation that uses another library class > lists those library classes in the library INF. This is > a way a module can inherit the use of a second lib without > listing the second lib in the module INF. This is the > type of inheritance that is supported by the EDK II build > system and the EDK II meta data files. >=20 > What is not supported today is indirect inheritance from > the libclass .h file itself. There is no mechanism for > the libclass .h file to declare it is using another library > class. This would require extensions to the meta data > files to declare this type dependency. >=20 > How does the build system know to add DebugCommonLib to > the link command for a module that only lists DebugLib > in its library classes section? What if there is a > DebugLib implementation of the DebugLib class that > does not depend on DebugCommonLib. The module link > will then fail when the module writer followed all the > rules. This is why including a libclass from another > libclass is bad idea. >=20 > Mike >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On >> Behalf Of Laszlo Ersek >> Sent: Tuesday, May 12, 2020 2:51 AM >> To: Kinney, Michael D ; >> Vitaly Cheptsov ; >> devel@edk2.groups.io >> Cc: Andrew Fish ; Marvin H=C3=A4user >> ; Gao, Liming >> ; Gao, Zhichao >> >> Subject: Re: [edk2-devel] [PATCH V4 00/27] Disabling >> safe string constraint assertions >>=20 >> On 05/12/20 00:40, Kinney, Michael D wrote: >>> Vitaly, >>>=20 >>> Thank you for the contribution. >>>=20 >>> There are a couple points about this approach that >> need to be discussed. >>>=20 >>> You have included the from >>> MdePkg/Include/Library/DebugLib.h. >>=20 >> Right, I've noticed it. I agree it's unusual. I didn't >> think it was wrong. >>=20 >>> It is very rare for a >>> lib class to include another lib class. This means >> that a module >>> that has a dependency on the DebugLib class inherits >> a hidden >>> dependency on the DebugCommonLib class. >>=20 >> I agree. >>=20 >> I think it should be fine. Any header H1 should include >> such further >> headers H2, H3, ... Hn that are required for making the >> interfaces >> declared in H1 usable in client modules. >>=20 >>> For module INF files, >>> we require the INF file to list all the lib classes >> that the >>> module sources directly use. >>=20 >> Yes, keyword being "directly". >>=20 >>> Since a module that uses the >>> DebugLib uses the ASSERT() and DEBUG() macros, all >> the APIs >>> that the ASSERT() and DEBUG() macros use are also >> directly >>> used by the module. >>=20 >> I believe this is where I disagree. The replacement >> texts of the >> ASSERT() and DEBUG() function-like macros are internals >> of the >> DebugLib.h lib class header, in my opinion. Those >> internals place >> requirements on specific DebugLib instances, not on >> DebugLib class >> consumers. >>=20 >> In other words, when writing a new DebugLib instance, >> the implementor >> has to ensure that the ASSERT() and DEBUG() macros, as >> defined in the >> DebugLib class header, will continue working in >> DebugLib consumer >> modules. The implementor may then choose to make the >> new DebugLib >> instance dependent on the (singleton) DebugCommonLib >> instance, for >> example. (This is being done in patches #3, #4, #16, >> maybe more.) The >> DebugLib consumer module will inherit that dependency, >> and everything >> will work. >>=20 >> Just because ASSERT() and DEBUG() are function-like >> macros and not >> actual functions, I don't think the INF file >> requirements in >> DebugLib-consumer modules should change. >>=20 >>> With this patch series, these macros >>> now use the DebugCommonLib class APIs, which means >> any module >>> that uses the DebugLib also directly uses the >> DebugCommonLib. >>=20 >> In my opinion: indirectly. >>=20 >> Thanks, >> Laszlo >>=20 >>=20 >>=20 >=20 --Apple-Mail=_B8EF74E7-4C13-42B5-A12B-6102CAE97DAF Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEsLABAI5Y5VbvBdmpL8K2O86Eyz4FAl668a4ACgkQL8K2O86E yz7YgA/8DX8UBgV+0ZqIWpQKZ5ejg66ueEa7+YZZvzoaXU/dHvRzSzc7qTme6Zvo VUBkQbG8U1A/1s9hYKdfx8ealIQLIr1QoovQK/XhakEB8DtnyDs/ixAy5gHBm/zQ 2M1P9Sf/QxJxwAjKNE+d20doxq7pTRseH056cA4xqNWCdgL58lXQ00k1mHTWGYSA K8y3pBWSlNXAPI1RFPhDgNvIwU3ysa0E8mgXCGSLPMP/LM0kzaz3ikS8fchqSaHg bIz0IvpCyp5QlPhw8TO1SpcgIxdBziDtv/lyUoAdTIOkAB275D3tETwS2pD42gVS SmYYWXzoAB1hfrK8gd/i4ytz69JcmW2FhJ+1qapJPhaTUWwI8ZUW7IGf3EUbSzwQ +HDqVA/pnwDP6SlVAE3RU+YGdfyGCa0Pf9l0EAIvV3Iix0H86G8w5i/vHsODGFwV sllJyZPAPlBhufw7W5A+hustzOZXFmOiZvYFjC6puFJl+r4uKq3eGlIka6n9WgGH yGR6h9lPuKmvASpJBIpZ0rF4mcNuOZMmEjp7uUabZkBWbbXWO/vM3uEJs5CVuA23 /LkerZnFiFBXfXYr95DYwMScar8MpAgK94XcwsSpd6GOU3jzphWTkSws+2ZbGuVz FgETC9C7pOsZI1GvP3PTSSav59Oqx1JtWNfrXpOFOJ/a5o67kgg= =Orvc -----END PGP SIGNATURE----- --Apple-Mail=_B8EF74E7-4C13-42B5-A12B-6102CAE97DAF--