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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/8/20 3:09 AM, Gao, Zhichao wrote:
> 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.

OK, thanks Zhichao.

> 
> 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.
>>>>
>>>>
>>
>>
>> 
>