Re: [edk2-devel] [PATCH V4 00/27] Disabling safe string constraint assertions

["Michael D Kinney" <michael.d.kinney@intel.com>]

Laszlo,

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.

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.

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.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Laszlo Ersek
> Sent: Tuesday, May 12, 2020 2:51 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> Vitaly Cheptsov <cheptsov@ispras.ru>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Marvin Häuser
> <mhaeuser@outlook.de>; Gao, Liming
> <liming.gao@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH V4 00/27] Disabling
> safe string constraint assertions
> 
> On 05/12/20 00:40, Kinney, Michael D wrote:
> > Vitaly,
> >
> > Thank you for the contribution.
> >
> > There are a couple points about this approach that
> need to be discussed.
> >
> > You have included the <Library/DebugCommonLib.h> from
> > MdePkg/Include/Library/DebugLib.h.
> 
> Right, I've noticed it. I agree it's unusual. I didn't
> think it was wrong.
> 
> > 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.
> 
> I agree.
> 
> 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.
> 
> > For module INF files,
> > we require the INF file to list all the lib classes
> that the
> > module sources directly use.
> 
> Yes, keyword being "directly".
> 
> > 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> > 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.
> 
> In my opinion: indirectly.
> 
> Thanks,
> Laszlo
> 
> 
>