Re: [edk2-devel] [PATCH] EmbeddedPkg/TimeBaseLib: remove the SEC_PER_MONTH, SEC_PER_YEAR macros

["Laszlo Ersek" <lersek@redhat.com>]

On 09/07/20 18:22, Leif Lindholm wrote:
> On Mon, Sep 07, 2020 at 10:23:08 +0800, xiewenyi (A) wrote:
>> On 2020/9/5 1:04, Philippe Mathieu-Daudé wrote:
>>> On 9/4/20 6:42 PM, Leif Lindholm wrote:
>>>> Ming bounced, adding xiewenyi2.
>>>>
>>>> On Fri, Sep 04, 2020 at 17:16:44 +0100, Leif Lindholm wrote:
>>>>> +Ming
>>>>>
>>>>> On Fri, Sep 04, 2020 at 17:45:41 +0200, Laszlo Ersek wrote:
>>>>>> The SEC_PER_MONTH and SEC_PER_YEAR macros are wrong: they both evaluate to
>>>>>> 0 (of type "int"). They are also unused (they could never be used for
>>>>>> division, for example); so remove them. The macros were originally
>>>>>> introduced in commit 0f4386e775c7 ("ArmPlatformPkg/PL031RealTimeClockLib:
>>>>>> Implement PL031 RTC drive", 2011-06-11).
>>>>>>
>>>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>>>>> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>>>>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>> Reported-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>>>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>>>
>>>>> Hmm, these are used in a couple of Hisilicon libraries in
>>>>> edk2-platforms. Cleary, they are not now functioning as expected (but
>>>>> they never invoke these macros for division).
>>>>>
>>
>> Hi,
>>
>> I search for these two macros but don't find them in Hisilicon, and also I try to
>> compile D06, D05, D03 in Hisilicon after deleting these two macros, there's no
>> compile error.
>>
>> So I think it's not used in Hisilicon libraries.
> 
> Err, ignore me, I somehow got the impression we were deleting all of
> the SEC_PER_ macros.
> Go ahead:
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>

Thanks! (To Phil as well)!

Merged as commit f94345d9eae1, via
<https://github.com/tianocore/edk2/pull/919>.

Laszlo

> 
>>>>> We should fix this, but possibly by correcting the macros instead of
>>>>> deleting them?
>>>
>>> I'm not sure how to fix, not all months have 30 days...
>>>
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>>
>>>>>
>>>>> /
>>>>>     Leif (goes back to now hopefuly lit barbecue)
>>>>>
>>>>>> ---
>>>>>>  EmbeddedPkg/Include/Library/TimeBaseLib.h | 2 --
>>>>>>  1 file changed, 2 deletions(-)
>>>>>>
>>>>>> diff --git a/EmbeddedPkg/Include/Library/TimeBaseLib.h b/EmbeddedPkg/Include/Library/TimeBaseLib.h
>>>>>> index ee2f191d985b..3c2d3660c66c 100644
>>>>>> --- a/EmbeddedPkg/Include/Library/TimeBaseLib.h
>>>>>> +++ b/EmbeddedPkg/Include/Library/TimeBaseLib.h
>>>>>> @@ -51,8 +51,6 @@
>>>>>>  #define SEC_PER_MIN                                     ((UINTN)    60)
>>>>>>  #define SEC_PER_HOUR                                    ((UINTN)  3600)
>>>>>>  #define SEC_PER_DAY                                     ((UINTN) 86400)
>>>>>> -#define SEC_PER_MONTH                                   ((UINTN)  2,592,000)
>>>>>> -#define SEC_PER_YEAR                                    ((UINTN) 31,536,000)
>>>>>>  
>>>>>>  BOOLEAN
>>>>>>  EFIAPI
>>>>>> -- 
>>>>>> 2.19.1.3.g30247aa5d201
>>>>>>
>>>>
>>>
>>>
>>
> 
> 
>