[edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables
Michael D Kinney
michael.d.kinney at intel.com
Fri Mar 24 15:54:08 UTC 2023
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 <mikuback at linux.microsoft.com>
> Sent: Friday, March 24, 2023 8:51 AM
> To: devel at edk2.groups.io; Kinney, Michael D <michael.d.kinney at intel.com>; Erich McMillan <emcmillan at microsoft.com>
> Cc: Dong, Eric <eric.dong at intel.com>; Kumar, Rahul R <rahul.r.kumar at intel.com>; Ni, Ray <ray.ni at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> >>>> Michael Kubacki
> >>>> Sent: Friday, March 10, 2023 10:43 AM
> >>>> To: devel at edk2.groups.io
> >>>> Cc: Dong, Eric <eric.dong at intel.com>; Erich McMillan
> >>>> <emcmillan at microsoft.com>; Kinney, Michael D
> >>>> <michael.d.kinney at intel.com>; Michael Kubacki
> >>>> <mikuback at linux.microsoft.com>; Kumar, Rahul R
> >>>> <rahul.r.kumar at intel.com>; Ni, Ray <ray.ni at intel.com>
> >>>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally
> >>>> uninitialized variables
> >>>>
> >>>> From: Michael Kubacki <michael.kubacki at microsoft.com>
> >>>>
> >>>> Fixes CodeQL alerts for CWE-457:
> >>>> https://cwe.mitre.org/data/definitions/457.html
> >>>>
> >>>> Cc: Eric Dong <eric.dong at intel.com>
> >>>> Cc: Erich McMillan <emcmillan at microsoft.com>
> >>>> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> >>>> Cc: Michael Kubacki <mikuback at linux.microsoft.com>
> >>>> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> >>>> Cc: Ray Ni <ray.ni at intel.com>
> >>>> Co-authored-by: Erich McMillan <emcmillan at microsoft.com>
> >>>> Signed-off-by: Michael Kubacki <michael.kubacki at 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+owner at edk2.groups.io
> >>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> >>>> [michael.d.kinney at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list