On 12/07/17 09:46, Ard Biesheuvel wrote: > On 7 December 2017 at 07:48, Liming Gao <[email protected]> wrote: >> From: Michael Kinney <[email protected]> >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=573 >> https://bugzilla.tianocore.org/show_bug.cgi?id=796 >> >> The same issue is reported again by GCC. Resend this patch again. >> This patch renames the duplicated function name to fix it. >> >> The SecPeiDebugAgentLib uses the global variable >> mMemoryDiscoveredNotifyList for a PPI notification on >> the Memory Discovered PPI. This same variable name is >> used in the DxeIplPeim for the same PPI notification. >> >> The XCODE5 tool chain detects this duplicate symbol >> when the OVMF platform is built with the flag >> -D SOURCE_DEBUG_ENABLE. >> >> The fix is to rename this global variable in the >> SecPeiDebugAgentLib library. >> > > No, the fix is to make it STATIC unless it *needs* to be a global. > Is that the case here?
I agree with you (of course), but Mike explained earlier (if I recall correctly -- and perhaps you remember too) that giving internal linkage to global variables (i.e., making them STATIC) messes either with debuggability under VS, or else defeats "GLOBAL_REMOVE_IF_UNREFERENCED". (I'm not sure which one of the two.) So, I've settled on considering "extern by default" just another peculiarity of edk2. *shrug* I'm just glad -fno-common catches bugs like this! Reviewed-by: Laszlo Ersek <[email protected]> (Obviously I'm not trying to dismiss your objection at all! Just stating my view. If the patch is changed to STATIC, I'll R-b that version *more happily* than this one.) Thanks! Laszlo > > >> Cc: Andrew Fish <[email protected]> >> Cc: Jeff Fan <[email protected]> >> Cc: Hao Wu <[email protected]> >> Cc: Laszlo Ersek <[email protected]> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Michael D Kinney <[email protected]> >> Reviewed-by: Jeff Fan <[email protected]> >> --- >> .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c | 4 >> ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git >> a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> >> b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> index b717e33..9f5223a 100644 >> --- >> a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> +++ >> b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> @@ -32,7 +32,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR >> mVectorHandoffInf >> } >> }; >> >> -GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_NOTIFY_DESCRIPTOR >> mMemoryDiscoveredNotifyList[1] = { >> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_NOTIFY_DESCRIPTOR >> mDebugAgentMemoryDiscoveredNotifyList[1] = { >> { >> (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | >> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), >> &gEfiPeiMemoryDiscoveredPpiGuid, >> @@ -554,7 +554,7 @@ InitializeDebugAgent ( >> // Register for a callback once memory has been initialized. >> // If memery has been ready, the callback funtion will be invoked >> immediately >> // >> - Status = PeiServicesNotifyPpi (&mMemoryDiscoveredNotifyList[0]); >> + Status = PeiServicesNotifyPpi >> (&mDebugAgentMemoryDiscoveredNotifyList[0]); >> if (EFI_ERROR (Status)) { >> DEBUG ((EFI_D_ERROR, "DebugAgent: Failed to register memory >> discovered callback function!\n")); >> CpuDeadLoop (); >> -- >> 2.6.3.windows.1 >> >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel >> GitPatchExtractor 1.1 >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
