Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables

["Michael Kubacki" <mikuback@linux.microsoft.com>]

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.dong@intel.com>; Erich McMillan 
>>>> <emcmillan@microsoft.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Michael Kubacki 
>>>> <mikuback@linux.microsoft.com>; Kumar, Rahul R
>>>> <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>
>>>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally 
>>>> uninitialized variables
>>>>
>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> Fixes CodeQL alerts for CWE-457:
>>>> https://cwe.mitre.org/data/definitions/457.html
>>>>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Erich McMillan <emcmillan@microsoft.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
>>>> Signed-off-by: Michael Kubacki <michael.kubacki@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@edk2.groups.io
>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub 
>>>> [michael.d.kinney@intel.com]
>>>> -=-=-=-=-=-=
>>>>
>>>
>>>
>>>
>>> 
>>>
>>>