Re: [edk2-devel] [PATCH V3 1/8] CryptoPkg/CryptoDxe: Add function to indicate the deprecated algorithm

["Gao, Zhichao" <zhichao.gao@intel.com>]

Thanks for point out the typo. I would fix it.
About the align, see below.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Philippe
> Mathieu-Daudé
> Sent: Thursday, May 7, 2020 3:48 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [PATCH V3 1/8] CryptoPkg/CryptoDxe: Add function to
> indicate the deprecated algorithm
> 
> On Thu, May 7, 2020 at 9:45 AM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
> >
> > On 5/7/20 1:57 AM, Gao, Zhichao wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1898
> > >
> > > Add a internal worker function to indicate the deprecated functions.
> > > It would print out debug messages and asserts ot inform the consumer
> >
> > Typo "to inform".
> >
> > > they are using a deprecated function.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > ---
> > >   CryptoPkg/Driver/Crypto.c | 18 ++++++++++++++++++
> > >   1 file changed, 18 insertions(+)
> > >
> > > diff --git a/CryptoPkg/Driver/Crypto.c b/CryptoPkg/Driver/Crypto.c
> > > index 35bf2d3d92..05ad4b5d55 100644
> > > --- a/CryptoPkg/Driver/Crypto.c
> > > +++ b/CryptoPkg/Driver/Crypto.c
> > > @@ -86,6 +86,24 @@ BaseCryptLibServciceNotEnabled (
> > >     ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
> > >   }
> > >
> > > +/**
> > > +  Internal worker function that prints a debug message and asserts
> > > +if a call is
> > > +  made to a BaseCryptLib function that is deprecated and unsupported any
> longer.
> > > +
> > > +  @param[in]  FunctionName  Null-terminated ASCII string that is the name
> of an
> > > +                            EDK II Crypto service.
> > > +
> > > +**/
> > > +static
> > > +VOID
> > > +BaseCryptLibServciceDeprecated (
> 
> Oh also another typo (extra 'c'):
> 
> "BaseCryptLibServiceDeprecated"
> 
> Maybe rename as "BaseCryptLibDeprecatedService"?
> 
> > > +  IN CONST CHAR8  *FunctionName
> >
> > Why extra space alignment?

See the CCS spec 2.1, 5.7.17:
5.7.1.7 The next lines contain parameters.
Each line will contain a single argument and will start indented two spaces (one tab stop). Type
and argument columns should be aligned to maximize readability and should include
appropriate spacing to ensure this alignment. No comments are allowed in this region.
Parameters are documented clearly in the function header comment block.

Thanks,
Zhichao

> >
> > Otherwise:
> > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> >
> > > +  )
> > > +{
> > > +  DEBUG ((DEBUG_ERROR, "[%a] Function %a() is deprecated and
> > > +unsupported any longer\n", gEfiCallerBaseName, FunctionName));
> > > +  ASSERT_EFI_ERROR (EFI_UNSUPPORTED); }
> > > +
> > >   /**
> > >     Returns the version of the EDK II Crypto Protocol.
> > >
> > >
> 
> 
>