On 04/26/19 10:30, Wu, Hao A wrote:
>> -----Original Message-----
>> From: Gao, Zhichao
>> Sent: Friday, April 26, 2019 4:12 PM
>> To: Wu, Hao A; [email protected]; Laszlo Ersek
>> Cc: Laszlo Ersek; Wang, Jian J; Ni, Ray; Zeng, Star; Gao, Liming; Bi, Dandan
>> Subject: RE: [edk2-devel] [PATCH V3 5/8]
>> MdeModulePkg/FirmwarePerformanceDxe: make global variable static
>>
>>
>>
>>> -----Original Message-----
>>> From: Wu, Hao A
>>> Sent: Friday, April 26, 2019 1:58 PM
>>> To: [email protected]; Gao, Zhichao <[email protected]>; Laszlo
>>> Ersek <[email protected]>
>>> Cc: Laszlo Ersek <[email protected]>; Wang, Jian J <[email protected]>;
>>> Ni, Ray <[email protected]>; Zeng, Star <[email protected]>; Gao, Liming
>>> <[email protected]>; Bi, Dandan <[email protected]>
>>> Subject: RE: [edk2-devel] [PATCH V3 5/8]
>>> MdeModulePkg/FirmwarePerformanceDxe: make global variable static
>>>
>>>> -----Original Message-----
>>>> From: [email protected] [mailto:[email protected]] On Behalf Of
>>>> Gao, Zhichao
>>>> Sent: Friday, April 26, 2019 9:04 AM
>>>> To: [email protected]
>>>> Cc: Laszlo Ersek; Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao,
>>>> Liming; Bi, Dandan
>>>> Subject: [edk2-devel] [PATCH V3 5/8]
>>>> MdeModulePkg/FirmwarePerformanceDxe: make global variable static
>>>
>>> After checking Laszlo's comment on V2:
>>>
>>> '''
>>> (1) in all of the subject lines, please replace
>>>
>>>   Decrease the name collisions
>>>
>>> with
>>>
>>>   make mExitBootServicesEvent STATIC
>>> '''
>>>
>>> I prefer the one suggested by Laszlo, the proposed subject here gives me
>>> the feeling of changing all the global variables in the module.
>>>
>>> However, I am okay if you stick with the proposed subject. Anyway,
>>> Reviewed-by: Hao Wu <[email protected]>
>>
>> I want to use that subject too. But the actual variable name '
>> mExitBootServicesEvent' is too long. If the changed driver name is also long
>> such as patch #5, then the subject would be out of size. And the actual 
>> changed
>> global variable is mentioned in the commit message. That is why I didn't take
>> Laszlo's suggestion.
> 
> Got it.
> 
> Just my opinion, the 72 characters limit for subject can be violated in
> special cases. And I believe this is one of the cases when keeping the
> title short makes it misleading.

I agree.

But, I think three versions of this patch series should be plenty :) So
I'm not requesting a repost.

Thanks
Laszlo

> 
> Best Regards,
> Hao Wu
> 
>>
>> Thanks,
>> Zhichao
>>
>>>
>>> Best Regards,
>>> Hao Wu
>>>
>>>>
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1740
>>>>
>>>> Add a 'static' storage-class specifier to the global variables that
>>>> only used in a single file to minimize the name collisions.
>>>> This is only for the variable named 'mExitBootServicesEvent'.
>>>>
>>>> Cc: Laszlo Ersek <[email protected]>
>>>> Cc: Jian J Wang <[email protected]>
>>>> Cc: Hao Wu <[email protected]>
>>>> Cc: Ray Ni <[email protected]>
>>>> Cc: Star Zeng <[email protected]>
>>>> Cc: Liming Gao <[email protected]>
>>>> Cc: Dandan Bi <[email protected]>
>>>> Signed-off-by: Zhichao Gao <[email protected]>
>>>> ---
>>>>  .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c  | 4
>>>> ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git
>>>>
>>> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firm
>>> wa
>>>> rePerformanceDxe.c
>>>>
>>> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firm
>>> wa
>>>> rePerformanceDxe.c
>>>> index 9713048f1f..61a7704b37 100644
>>>> ---
>>>>
>>> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firm
>>> wa
>>>> rePerformanceDxe.c
>>>> +++
>>>>
>>> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firm
>>> wa
>>>> rePerformanceDxe.c
>>>> @@ -5,7 +5,7 @@
>>>>    for Firmware Basic Boot Performance Record and other boot
>>>> performance records,
>>>>    and install FPDT to ACPI table.
>>>>
>>>> -  Copyright (c) 2011 - 2018, Intel Corporation. All rights
>>>> reserved.<BR>
>>>> +  Copyright (c) 2011 - 2019, Intel Corporation. All rights
>>>> + reserved.<BR>
>>>>    SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>
>>>>  **/
>>>> @@ -40,7 +40,7 @@ EFI_RSC_HANDLER_PROTOCOL
>>> *mRscHandlerProtocol
>>>> = NULL;
>>>>  BOOLEAN                     mLockBoxReady = FALSE;
>>>>  EFI_EVENT                   mReadyToBootEvent;
>>>>  EFI_EVENT                   mLegacyBootEvent;
>>>> -EFI_EVENT                   mExitBootServicesEvent;
>>>> +static EFI_EVENT            mExitBootServicesEvent;
>>>>  UINTN                       mFirmwarePerformanceTableTemplateKey  = 0;
>>>>  BOOLEAN                     mDxeCoreReportStatusCodeEnable = FALSE;
>>>>
>>>> --
>>>> 2.21.0.windows.1
>>>>
>>>>
>>>> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39662): https://edk2.groups.io/g/devel/message/39662
Mute This Topic: https://groups.io/mt/31348681/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to