Re: [edk2-devel] [PATCH V3 5/8] MdeModulePkg/FirmwarePerformanceDxe: make global variable static

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

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; devel@edk2.groups.io; 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: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Laszlo
>>> Ersek <lersek@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>;
>>> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
>>> <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>
>>> Subject: RE: [edk2-devel] [PATCH V3 5/8]
>>> MdeModulePkg/FirmwarePerformanceDxe: make global variable static
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>>> Gao, Zhichao
>>>> Sent: Friday, April 26, 2019 9:04 AM
>>>> To: devel@edk2.groups.io
>>>> 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 <hao.a.wu@intel.com>
>>
>> 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 <lersek@redhat.com>
>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>> Cc: Dandan Bi <dandan.bi@intel.com>
>>>> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>>>> ---
>>>>  .../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
>>>>
>>>>
>>>> 
>