Laszlo: > -----Original Message----- > From: [email protected] <[email protected]> On Behalf Of Laszlo Ersek > Sent: Wednesday, August 19, 2020 5:24 PM > To: Tom Lendacky <[email protected]>; [email protected] > Cc: Gao, Liming <[email protected]>; Dong, Eric <[email protected]>; Ni, > Ray <[email protected]>; Kumar, Rahul1 > <[email protected]> > Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize > the DoDecrement variable > > On 08/18/20 15:10, Tom Lendacky wrote: > > From: Tom Lendacky <[email protected]> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901 > > > > The DoDecrement variable in ApWakeupFunction () wasn't always being > > initialized. Update the code to always fully initialize it. > > > > Cc: Eric Dong <[email protected]> > > Cc: Ray Ni <[email protected]> > > Cc: Laszlo Ersek <[email protected]> > > Cc: Rahul Kumar <[email protected]> > > Signed-off-by: Tom Lendacky <[email protected]> > > --- > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > index 90416c81b616..e24bdc64f930 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > @@ -885,9 +885,7 @@ ApWakeupFunction ( > > UINT64 Status; > > BOOLEAN DoDecrement; > > > > - if (CpuMpData->InitFlag == ApInitConfig) { > > - DoDecrement = TRUE; > > - } > > + DoDecrement = (CpuMpData->InitFlag == ApInitConfig) ? TRUE : > > FALSE; > > > > while (TRUE) { > > Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB); > > > > Not that I want to obsess about style, but > > (condition) ? TRUE : FALSE > > is an anti-patter that's similar to > > (condition) == TRUE > > Instead, I suggest: > > DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig); > > (The (BOOLEAN) cast is necessary, or at least used to be necessary, > becasue the == operator returns "int" (INT32), but BOOLEAN (i.e., the > type of "DoDecrement") is UINT8 -- and some VS toolchains perceive (or > used to perceive) this implicit conversion as a "potential loss of > precision". That warning is of course bogus, as the == operator only > produces 0 or 1, each of which values fits comfortably into a UINT8. But > still the explicit (BOOLEAN) cast is how we suppress the warning.)
I agree this style is simpler than before. > > Different question: who's supposed to merge (v2 of) this? Per > "Maintainers.txt", it should be Eric or Ray; OTOH, maybe the fix is > urgent (build failure with CLANGPDB) and anyone with push access could > qualify. This fix needs to catch this stable tag. Once the package maintainer reviews it, I will merge it. Thanks Liming > > Thanks, > Laszlo > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#64442): https://edk2.groups.io/g/devel/message/64442 Mute This Topic: https://groups.io/mt/76264245/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
