Laszlo: This is a better way. The bug https://bugzilla.tianocore.org/show_bug.cgi?id=410 only lists GetBestLanguage() API. I am not sure whether they are similar cases in edk2. If have, we had better fix them together.
Thanks Liming >-----Original Message----- >From: Laszlo Ersek [mailto:[email protected]] >Sent: Monday, September 25, 2017 4:13 PM >To: Gao, Liming <[email protected]>; Marvin Häuser ><[email protected]>; [email protected] >Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in >CLANG38 toolchain > >On 09/25/17 06:59, Gao, Liming wrote: >> Marvin: >> My concern is that the fix is an incompatible change. It requires to modify >library API. In fact, this is an undefined behavior. But, current compiler (VS, >GCC and Clang) makes it work. So, I prefer to keep the code as-is, and disable >this warning first. If you find any real issue, we can return back and figure >out >the solution. > >I think we can draw a parallel here to GetVariable() and GetVariable2(). >GetVariable() is now unavailable if DISABLE_NEW_DEPRECATED_INTERFACES >is >defined. > >I think the right solution would be to >- introduce GetBestLanguage2(), >- migrate all the current call sites to GetBestLanguage2() -- I counted > 18, and the updates should be easy --, >- and then make GetVariable() conditional on > not-DISABLE_NEW_DEPRECATED_INTERFACES. > > >CHAR8 * >EFIAPI >GetBestLanguage2 ( > IN CONST CHAR8 *SupportedLanguages, > IN INTN Iso639Language, > ... > ); > >I don't feel strongly about this question, I just think technically this >would be best. CLANG warnings are valuable. > >Thanks >Laszlo > >>> -----Original Message----- >>> From: edk2-devel [mailto:[email protected]] On Behalf Of >>> Marvin H?user >>> Sent: Friday, September 22, 2017 11:53 PM >>> To: [email protected] >>> Cc: Gao, Liming <[email protected]> >>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 >in >>> CLANG38 toolchain >>> >>> Hey, >>> >>> I just noticed this patch as it recently has been pushed. I found this has >been a >>> reaction to https://bugzilla.tianocore.org/show_bug.cgi?id=410 >>> Though as Clang correctly detected, this is Undefined Behavior per the C >>> specification, so why was the warning hidden? >>> In context of the issue in UefiLib, providing the first element of the VA >>> list >as a >>> prototyped argument, would have solved the issue without UB. >>> >>> Do you wish such a patch? >>> >>> Thanks, >>> Marvin. >>> >>>> -----Original Message----- >>>> From: edk2-devel [mailto:[email protected]] On Behalf Of >>>> Gao, Liming >>>> Sent: Monday, August 28, 2017 9:19 AM >>>> To: Shi, Steven <[email protected]>; [email protected] >>>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and >LLVM40 >>> in >>>> CLANG38 toolchain >>>> >>>> Reviewed-by: Liming Gao <[email protected]> >>>> >>>>> -----Original Message----- >>>>> From: Shi, Steven >>>>> Sent: Wednesday, August 23, 2017 3:01 PM >>>>> To: [email protected]; Gao, Liming <[email protected]> >>>>> Cc: Zhu, Yonghong <[email protected]>; Shi, Steven >>>>> <[email protected]> >>>>> Subject: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in >>> CLANG38 >>>>> toolchain >>>>> >>>>> From: "Shi, Steven" <[email protected]> >>>>> >>>>> Add LLVM39 and LLVM40 support in CLANG38 toolchain >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Steven Shi <[email protected]> >>>>> --- >>>>> BaseTools/Conf/tools_def.template | 5 +++-- >>>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/BaseTools/Conf/tools_def.template >>>>> b/BaseTools/Conf/tools_def.template >>>>> index 1fa3ca3..2f83341 100755 >>>>> --- a/BaseTools/Conf/tools_def.template >>>>> +++ b/BaseTools/Conf/tools_def.template >>>>> @@ -380,7 +380,8 @@ DEFINE SOURCERY_CYGWIN_TOOLS = >>>> /cygdrive/c/Program >>>>> Files/CodeSourcery/Sourcery G >>>>> # Intel(r) ACPI Compiler from >>>>> # https://acpica.org/downloads >>>>> # CLANG38 -Linux- Requires: >>>>> -# Clang v3.8 or later, LLVMgold plugin and >>>>> GNU binutils >2.26 >>>>> targeting x86_64-linux-gnu >>>>> +# Clang v3.8, LLVMgold plugin and GNU >>>>> binutils 2.26 >>>> targeting >>>>> x86_64-linux-gnu >>>>> +# Clang v3.9 or later, LLVMgold plugin and >>>>> GNU binutils >2.28 >>>>> targeting x86_64-linux-gnu >>>>> # Optional: >>>>> # Required to build platforms or ACPI tables: >>>>> # Intel(r) ACPI Compiler from >>>>> @@ -5512,7 +5513,7 @@ DEFINE CLANG38_X64_PREFIX = >>>>> ENV(CLANG38_BIN) >>>>> DEFINE CLANG38_IA32_TARGET = -target i686-pc-linux-gnu >>>>> DEFINE CLANG38_X64_TARGET = -target x86_64-pc-linux-gnu >>>>> >>>>> -DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) - >>> Wno- >>>>> empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno- >>> shift- >>>>> negative-value -Wno-parentheses-equality -Wno-unknown-pragmas - >>>> Wno- >>>>> tautological-constant-out-of-range-compare -Wno-incompatible-library- >>>>> redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx - >>>> msoft- >>>>> float -mno-implicit-float -ftrap- >>>>> function=undefined_behavior_has_been_optimized_away_by_clang - >>>>> funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno- >>>>> tautological-compare -Wno-unknown-warning-option >>>>> +DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) - >>>> Wno- >>>>> empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno- >>> shift- >>>>> negative-value -Wno-parentheses-equality -Wno-unknown-pragmas - >>>> Wno- >>>>> tautological-constant-out-of-range-compare -Wno-incompatible-library- >>>>> redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx - >>>> msoft- >>>>> float -mno-implicit-float -ftrap- >>>>> function=undefined_behavior_has_been_optimized_away_by_clang - >>>>> funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno- >>>>> tautological-compare -Wno-unknown-warning-option -Wno-varargs >>>>> >>>>> ########################### >>>>> # CLANG38 IA32 definitions >>>>> -- >>>>> 2.7.4 >>>> >>>> _______________________________________________ >>>> 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 >> _______________________________________________ >> 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
