I would prefer to follow convention of rest of edk2 sources. which is to not use the postfix 'u' for small integers.
Mike > -----Original Message----- > From: Michael Kubacki <mikub...@linux.microsoft.com> > Sent: Friday, March 24, 2023 8:51 AM > To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com>; > Erich McMillan <emcmil...@microsoft.com> > Cc: Dong, Eric <eric.d...@intel.com>; Kumar, Rahul R > <rahul.r.ku...@intel.com>; Ni, Ray <ray...@intel.com> > Subject: Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally > uninitialized variables > > Hi Mike, > > Would you like a change here? > > Thanks, > Michael > > On 3/21/2023 10:25 AM, Michael Kubacki wrote: > > I spoke to Erich offline and he mentioned that a previous coding > > practice he used specified unsigned integer literals when intended so he > > applied that here. > > > > Thanks, > > Michael > > > > On 3/10/2023 5:59 PM, Michael Kubacki wrote: > >> Erich introduced this particular change, he may be able to provide > >> more info. > >> > >> On 3/10/2023 3:03 PM, Michael D Kinney wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > >>>> Michael Kubacki > >>>> Sent: Friday, March 10, 2023 10:43 AM > >>>> To: devel@edk2.groups.io > >>>> Cc: Dong, Eric <eric.d...@intel.com>; Erich McMillan > >>>> <emcmil...@microsoft.com>; Kinney, Michael D > >>>> <michael.d.kin...@intel.com>; Michael Kubacki > >>>> <mikub...@linux.microsoft.com>; Kumar, Rahul R > >>>> <rahul.r.ku...@intel.com>; Ni, Ray <ray...@intel.com> > >>>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally > >>>> uninitialized variables > >>>> > >>>> From: Michael Kubacki <michael.kuba...@microsoft.com> > >>>> > >>>> Fixes CodeQL alerts for CWE-457: > >>>> https://cwe.mitre.org/data/definitions/457.html > >>>> > >>>> Cc: Eric Dong <eric.d...@intel.com> > >>>> Cc: Erich McMillan <emcmil...@microsoft.com> > >>>> Cc: Michael D Kinney <michael.d.kin...@intel.com> > >>>> Cc: Michael Kubacki <mikub...@linux.microsoft.com> > >>>> Cc: Rahul Kumar <rahul1.ku...@intel.com> > >>>> Cc: Ray Ni <ray...@intel.com> > >>>> Co-authored-by: Erich McMillan <emcmil...@microsoft.com> > >>>> Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > >>>> --- > >>>> UefiCpuPkg/CpuMpPei/CpuBist.c | 8 +++++++- > >>>> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 8 +++++++- > >>>> UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 ++++++++- > >>>> 3 files changed, 22 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuBist.c > >>>> b/UefiCpuPkg/CpuMpPei/CpuBist.c > >>>> index 7dc93cd784d4..122808139b87 100644 > >>>> --- a/UefiCpuPkg/CpuMpPei/CpuBist.c > >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuBist.c > >>>> @@ -175,7 +175,13 @@ CollectBistDataFromPpi ( > >>>> EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2; > >>>> EFI_SEC_PLATFORM_INFORMATION_CPU *CpuInstanceInHob; > >>>> > >>>> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, > >>>> &NumberOfEnabledProcessors); > >>>> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, > >>>> &NumberOfEnabledProcessors); > >>>> + ASSERT_EFI_ERROR (Status); > >>>> + > >>>> + if (EFI_ERROR (Status)) { > >>>> + NumberOfProcessors = 1u; > >>>> + NumberOfEnabledProcessors = 1u; > >>> > >>> Why is '1u' used instead of '1'? I don't see a lot of usage of the > >>> postfix > >>> unless the const needs to be forced to a larger bitwidth than default > >>> int type. > >>> > >>>> + } > >>>> > >>>> BistInformationSize = sizeof > >>>> (EFI_SEC_PLATFORM_INFORMATION_RECORD2) + > >>>> sizeof (EFI_SEC_PLATFORM_INFORMATION_CPU) > >>>> * NumberOfProcessors; > >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c > >>>> b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > >>>> index e7f1fe9f426c..a84304273168 100644 > >>>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c > >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > >>>> @@ -473,7 +473,13 @@ InitializeMpExceptionStackSwitchHandlers ( > >>>> return; > >>>> } > >>>> > >>>> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); > >>>> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); > >>>> + ASSERT_EFI_ERROR (Status); > >>>> + > >>>> + if (EFI_ERROR (Status)) { > >>>> + NumberOfProcessors = 1u; > >>>> + } > >>>> + > >>>> SwitchStackData = AllocatePages (EFI_SIZE_TO_PAGES > >>>> (NumberOfProcessors * sizeof > >>>> (EXCEPTION_STACK_SWITCH_CONTEXT))); > >>>> ASSERT (SwitchStackData != NULL); > >>>> ZeroMem (SwitchStackData, NumberOfProcessors * sizeof > >>>> (EXCEPTION_STACK_SWITCH_CONTEXT)); > >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c > >>>> b/UefiCpuPkg/CpuMpPei/CpuPaging.c > >>>> index 135422225340..1322fcb77f28 100644 > >>>> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > >>>> @@ -538,6 +538,7 @@ SetupStackGuardPage ( > >>>> UINTN NumberOfProcessors; > >>>> UINTN Bsp; > >>>> UINTN Index; > >>>> + EFI_STATUS Status; > >>>> > >>>> // > >>>> // One extra page at the bottom of the stack is needed for Guard > >>>> page. > >>>> @@ -547,7 +548,13 @@ SetupStackGuardPage ( > >>>> ASSERT (FALSE); > >>>> } > >>>> > >>>> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); > >>>> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); > >>>> + ASSERT_EFI_ERROR (Status); > >>>> + > >>>> + if (EFI_ERROR (Status)) { > >>>> + NumberOfProcessors = 1u; > >>>> + } > >>>> + > >>>> MpInitLibWhoAmI (&Bsp); > >>>> for (Index = 0; Index < NumberOfProcessors; ++Index) { > >>>> StackBase = 0; > >>>> -- > >>>> 2.39.2.windows.1 > >>>> > >>>> > >>>> > >>>> -=-=-=-=-=-= > >>>> Groups.io Links: You receive all messages sent to this group. > >>>> View/Reply Online (#101030): > >>>> https://edk2.groups.io/g/devel/message/101030 > >>>> Mute This Topic: https://groups.io/mt/97526805/1643496 > >>>> Group Owner: devel+ow...@edk2.groups.io > >>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub > >>>> [michael.d.kin...@intel.com] > >>>> -=-=-=-=-=-= > >>>> > >>> > >>> > >>> > >>> > >>> > >>> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101799): https://edk2.groups.io/g/devel/message/101799 Mute This Topic: https://groups.io/mt/97526805/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
