Thanks Marvin, I will fix according the comments. For 2), I don’t see assert
only code in my patch. The only allocation in this patch is below, which has
already been handled by if check.
if (mSmmInitialized == NULL) {
mSmmInitialized = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) *
mMaxNumberOfCpus);
}
ASSERT (mSmmInitialized != NULL);
if (mSmmInitialized == NULL) {
return;
}
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin H?user
Sent: Friday, February 10, 2023 6:00 PM
To: Wu; Wu, Jiaxin <jiaxin...@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v4 3/5] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM
Base Hob for SmBase info
Hi Jiaxin,
1) mSmmInitialized *must* be volatile. Your current code may cause anything,
from skipping waiting entirely (the loop is optimized away as the compiler
considers it free from side effects) to stalling (the memory access is
optimized away as the compiler considers it locally-immutable).
2) ASSERTs on memory allocation failures are one of the most terrible edk2
"paradigms".
3) Comparisons against boolean constants are not allowed by the code style spec.
Best regards,
Marvin
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100048): https://edk2.groups.io/g/devel/message/100048
Mute This Topic: https://groups.io/mt/96871374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
