> -----Original Message-----
> From: Laszlo Ersek [mailto:[email protected]]
> Sent: Tuesday, April 23, 2019 10:08 PM
> To: [email protected]; Gao, Zhichao <[email protected]>
> Cc: Kinney, Michael D <[email protected]>; Gao, Liming
> <[email protected]>; Bi, Dandan <[email protected]>
> Subject: Re: [edk2-devel] [PATCH 0/3] MdePkg/DebugLib: Change the global
> variable name
> 
> On 04/23/19 04:35, Gao, Zhichao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1740
> >
> > The DebugLib instances of DebugPortProtocol, ConOut and StdErr use a
> > global variable "mExitBootServicesEvent" which is in conflict with the
> > same variable in StatusCodeHandlerRuntimeDxe.inf.
> > That would cause a build error through GCC5. So change the name to the
> > "mDebugLibExitBootServicesEvent".
> >
> > Cc: Michael D Kinney <[email protected]>
> > Cc: Liming Gao <[email protected]>
> > Cc: Dandan Bi <[email protected]>
> >
> > Zhichao Gao (3):
> >   MdePkg/UefiDebugLibConOut: Change the global variable name
> >   MdePkg/UefiDebugLibStdErr: Change the global variable name
> >   MdePkg/UefiDebugLibDebugPortProtocol: Change the global variable
> > name
> >
> >  MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c       | 4 ++--
> >  .../UefiDebugLibDebugPortProtocol/DebugLibConstructor.c       | 4 ++--
> >  MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c       | 4 ++--
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >
> 
> The proper solution for this kind of error is to make as many as possible
> instances of "mExitBootServicesEvent" in edk2 STATIC.
> 
> See for example commit 7b13510f2a0a
> ("MdeModulePkg/BootMaintenanceManagerUiLib: hide library-internal
> symbol", 2016-05-17).
> 
> In particular, this patch renames three instances of mExitBootServicesEvent,
> but there are more:
> 
> -
> IntelFrameworkModulePkg/Library/SmmRuntimeDxeReportStatusCodeLibFr
> amework/SmmRuntimeDxeSupport.c
> -
> IntelFrameworkModulePkg/Universal/StatusCode/DatahubStatusCodeHandl
> erDxe/DatahubStatusCodeHandlerDxe.c
> - IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> -
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firmw
> arePerformanceDxe.c
> -
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.c
> 
> Based on a brief investigation, it seems like the "STATIC" approach should
> work for all 8 (eight) files above. But, minimally, STATIC should be employed
> with library instances.
> 
> I seem to remember that there used to be debugging issues with Visual
> Studio if global variables were made STATIC -- but I think that only applied 
> to
> old (no longer supported by edk2?) Visual Studio versions. If you can't use
> STATIC here, please at least explain why, in the commit messages.

Thanks for your particular comments. I used to receive a comment to suggest me 
to make some global variable as 'STATIC'. But I consider the variable as the 
component scope that the variable may be used by the other files in the 
component. I forgot to consider the namespace collisions.
Agree with you, the variable such as 'mExitBootServicesEvent' is only used to 
create an event and close and it is useless for other files. All such variables 
should add a 'STATIC' description for them. I would update all the files your 
mentioned to decrease the namespace collisions.

Thanks,
Zhichao

> 
> Thanks,
> Laszlo

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

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

Reply via email to