From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 23 Apr 2019 07:08:06 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AA28C309E97E; Tue, 23 Apr 2019 14:08:02 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-235.rdu2.redhat.com [10.10.120.235]) by smtp.corp.redhat.com (Postfix) with ESMTP id CEF4D60C81; Tue, 23 Apr 2019 14:07:57 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/3] MdePkg/DebugLib: Change the global variable name To: devel@edk2.groups.io, zhichao.gao@intel.com Cc: Michael D Kinney , Liming Gao , Dandan Bi References: <20190423023507.16952-1-zhichao.gao@intel.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 23 Apr 2019 16:07:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190423023507.16952-1-zhichao.gao@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Tue, 23 Apr 2019 14:08:06 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Liming Gao > Cc: Dandan Bi > > 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/SmmRuntimeDxeReportStatusCodeLibFramework/SmmRuntimeDxeSupport.c - IntelFrameworkModulePkg/Universal/StatusCode/DatahubStatusCodeHandlerDxe/DatahubStatusCodeHandlerDxe.c - IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c - MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c - MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.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, Laszlo