Reviewed-by: Star Zeng <star.z...@intel.com> To the attached patch.
Thanks, Star From: af...@apple.com [mailto:af...@apple.com] Sent: Monday, October 26, 2015 9:31 AM To: Zeng, Star Cc: edk2-devel-01 Subject: Re: [edk2] [MdeModulePkg] PeiMain PeiCore does not compile with Xcode 6.3.2 > On Oct 25, 2015, at 6:04 PM, Zeng, Star <star.z...@intel.com> wrote: > > On 2015/10/23 22:11, Andrew Fish wrote: >> >>> On Oct 23, 2015, at 12:53 AM, Zeng, Star <star.z...@intel.com> wrote: >>> >>> On 2015/10/22 23:54, Andrew Fish wrote: >>>> /Users/andrewfish/work/src/edk2/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c:682:28: >>>> error: for loop has empty body [-Werror,-Wempty-body] >>>> StackPointer ++); >>>> ^ >>>> /Users/andrewfish/work/src/edk2/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c:682:28: >>>> note: put the semicolon on a separate line to silence this warning >>>> 1 error generated. >>> >>> Do you have full code base and all package build about this warning? >>> I can some for loop like below in UefiLib.c AddUnicodeString2(). And there >>> maybe many other places have similar cases I guess since I never met this >>> build failure before. >>> >>> for (; *SupportedLanguages != 0 && *SupportedLanguages == ';'; >>> SupportedLanguages++); >>> >> >> That is legal >> >>> What's the real value of this build warning? >> >> -Wempty-body is designed to catch code bugs like: >> >> if (X); >> X++; >> >> In the above code X++ always runs. >> >> >>> And could you disable this warning the tool chain? >>> >> >> Yes any clang diagnostic that prints out the -W* you can do -Wno-* >> >> >> I realize my fix is bad. The error is really the next DEBUG() macro is >> indented like it is in a loop body. That is what is triggering the >> diagnostic. >> >> for (; *SupportedLanguages != 0 && *SupportedLanguages == ';'; >> SupportedLanguages++); >> WarningForLoopIndentation(); >> >> >> DEBUG_CODE_BEGIN (); >> UINT32 *StackPointer; >> >> for (StackPointer = (UINT32*)SecCoreData->StackBase; >> (StackPointer < (UINT32*)((UINTN)SecCoreData->StackBase + >>SecCoreData->StackSize)) \ >> && (*StackPointer == INIT_CAR_VALUE); >> StackPointer ++); >> >> DEBUG ((EFI_D_INFO, "Temp Stack : BaseAddress=0x%p Length=0x%X\n", >>SecCoreData->StackBase, (UINT32)SecCoreData->StackSize)); >> DEBUG ((EFI_D_INFO, "Temp Heap : BaseAddress=0x%p Length=0x%X\n", >>Private->HobList.Raw, (UINT32)((UINTN) >>Private->HobList.HandoffInformationTable->EfiFreeMemoryBottom - (UINTN) >>Private->HobList.Raw))); >> DEBUG ((EFI_D_INFO, "Total temporary memory: %d bytes.\n", >>(UINT32)SecCoreData->TemporaryRamSize)); >> DEBUG ((EFI_D_INFO, " temporary memory stack ever used: %d >>bytes.\n", >> (UINT32)(SecCoreData->StackSize - ((UINTN) StackPointer - >>(UINTN)SecCoreData->StackBase)) >> )); >> DEBUG ((EFI_D_INFO, " temporary memory heap used: %d >>bytes.\n", >> >>(UINT32)((UINTN)Private->HobList.HandoffInformationTable->EfiFreeMemoryBottom >>- (UINTN)Private->HobList.Raw) >> )); >> DEBUG_CODE_END (); >> >> Fixing the indentation to follow the coding standard makes the warning go >> away. That is the correct fix. > > Yes, I like this method to fix the indentation. > Could you help provide the formal patch? > Thanks, Andrew Fish > Thanks, > Star > >> >> Thanks, >> >> Andrew Fish >> >>> Thanks, >>> Star >>> >>>> >>>> This code change removes the warning. >>>> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c >>>> b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c >>>> index 7480b66..c563775 100644 >>>> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c >>>> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c >>>> @@ -679,7 +679,8 @@ PeiCheckAndSwitchStack ( >>>> for (StackPointer = (UINT32*)SecCoreData->StackBase; >>>> (StackPointer < (UINT32*)((UINTN)SecCoreData->StackBase + >>>>SecCoreData->StackSize)) \ >>>> && (*StackPointer == INIT_CAR_VALUE); >>>> - StackPointer ++); >>>> + StackPointer ++) >>>> + ; >>>> >>>> DEBUG ((EFI_D_INFO, "Temp Stack : BaseAddress=0x%p >>>>Length=0x%X\n", SecCoreData->StackBase, (UINT32)SecCoreData->StackSize)); >>>> DEBUG ((EFI_D_INFO, "Temp Heap : BaseAddress=0x%p >>>>Length=0x%X\n", Private->HobList.Raw, (UINT32)((UINTN) >>>>Private->HobList.HandoffInformationTable->EfiFreeMemoryBottom - (UINTN) >>>>Private->HobList.Raw))); >>>> >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Andrew Fish <af...@apple.com <mailto:af...@apple.com>> >>>> >>>> Thanks, >>>> >>>> Andrew Fish >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
