public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Oliver Smith-Denny" <osd@smith-denny.com>
To: Michael Kubacki <mikuback@linux.microsoft.com>, devel@edk2.groups.io
Cc: Erich McMillan <emcmillan@microsoft.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Ray Ni <ray.ni@intel.com>, Zhichao Gao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v7 09/12] ShellPkg: Fix conditionally uninitialized variables
Date: Tue, 28 Mar 2023 16:06:45 -0700	[thread overview]
Message-ID: <4fe27a2b-e892-cc67-7ea7-762089641384@smith-denny.com> (raw)
In-Reply-To: <99becc09-aa31-aded-6d3e-b48d74b86c49@linux.microsoft.com>

On 3/28/2023 12:24 PM, Michael Kubacki wrote:
> Hi Oliver,
> 
> Thanks for taking time to look through the series. I responded inline.
> 
> Thanks,
> Michael
> 
> On 3/28/2023 2:49 PM, Oliver Smith-Denny wrote:
>> A couple comments below, thanks!
>>
>> On 3/24/2023 3:30 PM, Michael Kubacki wrote:
>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>
>>> Fixes CodeQL alerts for CWE-457:
>>> https://cwe.mitre.org/data/definitions/457.html
>>>
>>> Cc: Erich McMillan <emcmillan@microsoft.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Zhichao Gao <zhichao.gao@intel.com>
>>> Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>> Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
>>> ---
>>>   ShellPkg/Application/Shell/Shell.c                          |  1 +
>>>   ShellPkg/Application/Shell/ShellProtocol.c                  | 60 
>>> ++++++++++----------
>>>   ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c  | 56 
>>> +++++++++---------
>>>   ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c          | 18 
>>> +++---
>>>   ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c |  9 ++-
>>>   ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c      | 14 +++--
>>>   ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c   | 17 
>>> ++++--
>>>   ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c      | 21 
>>> +++----
>>>   8 files changed, 107 insertions(+), 89 deletions(-)
>>>
>>> diff --git a/ShellPkg/Application/Shell/Shell.c 
>>> b/ShellPkg/Application/Shell/Shell.c
>>> index 0ae6e14a34bf..f95c799bb2a4 100644
>>> --- a/ShellPkg/Application/Shell/Shell.c
>>> +++ b/ShellPkg/Application/Shell/Shell.c
>>> @@ -1300,6 +1300,7 @@ DoStartupScript (
>>>     CHAR16         *FullFileStringPath;
>>>     UINTN          NewSize;
>>> +  CalleeStatus    = EFI_SUCCESS;
>>>     Key.UnicodeChar = CHAR_NULL;
>>>     Key.ScanCode    = 0;
>>> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c 
>>> b/ShellPkg/Application/Shell/ShellProtocol.c
>>> index e6d20ab16479..da8c31cb038a 100644
>>> --- a/ShellPkg/Application/Shell/ShellProtocol.c
>>> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
>>> @@ -735,50 +735,52 @@ EfiShellGetDeviceName (
>>>       //
>>>       // Now check the parent controller using this as the child.
>>>       //
>>> -    if (DeviceNameToReturn == NULL) {
>>> -      PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, 
>>> &ParentControllerCount, &ParentControllerBuffer);
>>> +    Status = PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, 
>>> &ParentControllerCount, &ParentControllerBuffer);
>>> +    if ((DeviceNameToReturn == NULL) && !EFI_ERROR (Status)) {
>>>         for (LoopVar = 0; LoopVar < ParentControllerCount; LoopVar++) {
>>> -        PARSE_HANDLE_DATABASE_UEFI_DRIVERS 
>>> (ParentControllerBuffer[LoopVar], &ParentDriverCount, 
>>> &ParentDriverBuffer);
>>> -        for (HandleCount = 0; HandleCount < ParentDriverCount; 
>>> HandleCount++) {
>>> -          //
>>> -          // try using that driver's component name with controller 
>>> and our driver as the child.
>>> -          //
>>> -          Status = gBS->OpenProtocol (
>>> -                          ParentDriverBuffer[HandleCount],
>>> -                          &gEfiComponentName2ProtocolGuid,
>>> -                          (VOID **)&CompName2,
>>> -                          gImageHandle,
>>> -                          NULL,
>>> -                          EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>> -                          );
>>> -          if (EFI_ERROR (Status)) {
>>> +        Status = PARSE_HANDLE_DATABASE_UEFI_DRIVERS 
>>> (ParentControllerBuffer[LoopVar], &ParentDriverCount, 
>>> &ParentDriverBuffer);
>>> +        if (!EFI_ERROR (Status)) {
>>> +          for (HandleCount = 0; HandleCount < ParentDriverCount; 
>>> HandleCount++) {
>>> +            //
>>> +            // try using that driver's component name with 
>>> controller and our driver as the child.
>>> +            //
>>>               Status = gBS->OpenProtocol (
>>>                               ParentDriverBuffer[HandleCount],
>>> -                            &gEfiComponentNameProtocolGuid,
>>> +                            &gEfiComponentName2ProtocolGuid,
>>>                               (VOID **)&CompName2,
>>>                               gImageHandle,
>>>                               NULL,
>>>                               EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>>                               );
>>> -          }
>>> +            if (EFI_ERROR (Status)) {
>>> +              Status = gBS->OpenProtocol (
>>> +                              ParentDriverBuffer[HandleCount],
>>> +                              &gEfiComponentNameProtocolGuid,
>>> +                              (VOID **)&CompName2,
>>> +                              gImageHandle,
>>> +                              NULL,
>>> +                              EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>> +                              );
>>> +            }
>>> +
>>> +            if (EFI_ERROR (Status)) {
>>> +              continue;
>>> +            }
>>> -          if (EFI_ERROR (Status)) {
>>> -            continue;
>>> +            Lang   = GetBestLanguageForDriver 
>>> (CompName2->SupportedLanguages, Language, FALSE);
>>> +            Status = CompName2->GetControllerName (CompName2, 
>>> ParentControllerBuffer[LoopVar], DeviceHandle, Lang, 
>>> &DeviceNameToReturn);
>>> +            FreePool (Lang);
>>> +            Lang = NULL;
>>> +            if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
>>> +              break;
>>> +            }
>>>             }
>>> -          Lang   = GetBestLanguageForDriver 
>>> (CompName2->SupportedLanguages, Language, FALSE);
>>> -          Status = CompName2->GetControllerName (CompName2, 
>>> ParentControllerBuffer[LoopVar], DeviceHandle, Lang, 
>>> &DeviceNameToReturn);
>>> -          FreePool (Lang);
>>> -          Lang = NULL;
>>> +          SHELL_FREE_NON_NULL (ParentDriverBuffer);
>>>             if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
>>>               break;
>>>             }
>>>           }
>>> -
>>> -        SHELL_FREE_NON_NULL (ParentDriverBuffer);
>>> -        if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
>>> -          break;
>>> -        }
>>>         }
>>>         SHELL_FREE_NON_NULL (ParentControllerBuffer);
>>> diff --git 
>>> a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c 
>>> b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
>>> index 36cf46fb2c38..4549cbde9b9a 100644
>>> --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
>>> +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
>>> @@ -1399,10 +1399,11 @@ ShellCommandCreateInitialMappingsAndPaths (
>>>     CHAR16                    *MapName;
>>>     SHELL_MAP_LIST            *MapListItem;
>>> -  SplitCurDir = NULL;
>>> -  MapName     = NULL;
>>> -  MapListItem = NULL;
>>> -  HandleList  = NULL;
>>> +  ConsistMappingTable = NULL;
>>> +  SplitCurDir         = NULL;
>>> +  MapName             = NULL;
>>> +  MapListItem         = NULL;
>>> +  HandleList          = NULL;
>>>     //
>>>     // Reset the static members back to zero
>>> @@ -1458,32 +1459,35 @@ ShellCommandCreateInitialMappingsAndPaths (
>>>       //
>>>       PerformQuickSort (DevicePathList, Count, sizeof 
>>> (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
>>> -    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
>>> -    //
>>> -    // Assign new Mappings to all...
>>> -    //
>>> -    for (Count = 0; HandleList[Count] != NULL; Count++) {
>>> +    if (!EFI_ERROR (ShellCommandConsistMappingInitialize 
>>> (&ConsistMappingTable))) {
>>>         //
>>> -      // Get default name first
>>> +      // Assign new Mappings to all...
>>>         //
>>> -      NewDefaultName = ShellCommandCreateNewMappingName 
>>> (MappingTypeFileSystem);
>>> -      ASSERT (NewDefaultName != NULL);
>>
>> Should we be checking for NewDefaultName to not be NULL before doing 
>> the below function call? Looks like 
>> ShellCommandAddMapItemAndUpdatePath does not do NULL checking and 
>> directly uses the Name (where it would fail in StrSize()). Similarly 
>> we then call FreePool on it, even if it is NULL.
>>
> Yes, but that's outside the scope of this patch. If someone were to fix 
> that, they should also address the other ~20 places in the file that use 
> asserts in place of checking null properly.

Fair enough :)
> 
> 
>>> -      Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, 
>>> DevicePathList[Count], 0, TRUE);
>>> -      ASSERT_EFI_ERROR (Status);
>>> -      FreePool (NewDefaultName);
>>> -
>>> -      //
>>> -      // Now do consistent name
>>> -      //
>>> -      NewConsistName = ShellCommandConsistMappingGenMappingName 
>>> (DevicePathList[Count], ConsistMappingTable);
>>> -      if (NewConsistName != NULL) {
>>> -        Status = ShellCommandAddMapItemAndUpdatePath 
>>> (NewConsistName, DevicePathList[Count], 0, FALSE);
>>> +      for (Count = 0; HandleList[Count] != NULL; Count++) {
>>> +        //
>>> +        // Get default name first
>>> +        //
>>> +        NewDefaultName = ShellCommandCreateNewMappingName 
>>> (MappingTypeFileSystem);
>>> +        ASSERT (NewDefaultName != NULL);
>>> +        Status = ShellCommandAddMapItemAndUpdatePath 
>>> (NewDefaultName, DevicePathList[Count], 0, TRUE);
>>>           ASSERT_EFI_ERROR (Status);
>>> -        FreePool (NewConsistName);
>>> +        FreePool (NewDefaultName);
>>> +
>>> +        //
>>> +        // Now do consistent name
>>> +        //
>>> +        NewConsistName = ShellCommandConsistMappingGenMappingName 
>>> (DevicePathList[Count], ConsistMappingTable);
>>> +        if (NewConsistName != NULL) {
>>> +          Status = ShellCommandAddMapItemAndUpdatePath 
>>> (NewConsistName, DevicePathList[Count], 0, FALSE);
>>> +          ASSERT_EFI_ERROR (Status);
>>> +          FreePool (NewConsistName);
>>> +        }
>>>         }
>>>       }
>>> -    ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
>>> +    if (ConsistMappingTable != NULL) {
>>> +      ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
>>> +    }
>>>       SHELL_FREE_NON_NULL (HandleList);
>>>       SHELL_FREE_NON_NULL (DevicePathList);
>>> @@ -1626,12 +1630,12 @@ ShellCommandUpdateMapping (
>>>       //
>>>       PerformQuickSort (DevicePathList, Count, sizeof 
>>> (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
>>> -    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
>>> +    Status = ShellCommandConsistMappingInitialize 
>>> (&ConsistMappingTable);
>>>       //
>>>       // Assign new Mappings to remainders
>>>       //
>>> -    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL 
>>> && !EFI_ERROR (Status); Count++) {
>>> +    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != 
>>> NULL; Count++) {
>>>         //
>>>         // Skip ones that already have
>>>         //
>>> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c 
>>> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
>>> index 97a4b57a932f..5329b559ba46 100644
>>> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
>>> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
>>> @@ -158,7 +158,10 @@ ShellCommandRunDblk (
>>>             ShellStatus = SHELL_INVALID_PARAMETER;
>>>           }
>>> -        ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE);
>>> +        if (EFI_ERROR (ShellConvertStringToUint64 (LbaString, &Lba, 
>>> TRUE, FALSE))) {
>>> +          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", LbaString);
>>> +          ShellStatus = SHELL_INVALID_PARAMETER;
>>> +        }
>>>         }
>>>         if (BlockCountString == NULL) {
>>> @@ -169,12 +172,13 @@ ShellCommandRunDblk (
>>>             ShellStatus = SHELL_INVALID_PARAMETER;
>>>           }
>>> -        ShellConvertStringToUint64 (BlockCountString, &BlockCount, 
>>> TRUE, FALSE);
>>> -        if (BlockCount > 0x10) {
>>> -          BlockCount = 0x10;
>>> -        } else if (BlockCount == 0) {
>>> -          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
>>> -          ShellStatus = SHELL_INVALID_PARAMETER;
>>> +        if (!EFI_ERROR (ShellConvertStringToUint64 
>>> (BlockCountString, &BlockCount, TRUE, FALSE))) {
>>> +          if (BlockCount > 0x10) {
>>> +            BlockCount = 0x10;
>>> +          } else if (BlockCount == 0) {
>>> +            ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
>>> +            ShellStatus = SHELL_INVALID_PARAMETER;
>>> +          }
>>>           }
>>>         }
>>> diff --git 
>>> a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c 
>>> b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
>>> index 8bf23a2076a1..72f8c087cb69 100644
>>> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
>>> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
>>> @@ -112,10 +112,13 @@ ShellCommandRunEfiDecompress (
>>>           if (ShellStatus == SHELL_SUCCESS) {
>>>             Status = FileHandleGetSize (InFileHandle, &Temp64Bit);
>>> -          ASSERT (Temp64Bit <= (UINT32)(-1));
>>> -          InSize = (UINTN)Temp64Bit;
>>>             ASSERT_EFI_ERROR (Status);
>>> -          InBuffer = AllocateZeroPool (InSize);
>>> +          if (!EFI_ERROR (Status)) {
>>
>> nit: If we got an EFI_ERROR from FileHandleGetSize, we will return 
>> EFI_OUT_OF_RESOURCES when we hit InBuffer == NULL below, which is not 
>> the real reason we failed. Perhaps we don't care, though.
>>
> True. The goal here was not to change that behavior.
> 
>>> +            ASSERT (Temp64Bit <= (UINT32)(-1));
>>> +            InSize   = (UINTN)Temp64Bit;
>>> +            InBuffer = AllocateZeroPool (InSize);
>>> +          }
>>> +
>>>             if (InBuffer == NULL) {
>>>               Status = EFI_OUT_OF_RESOURCES;
>>>             } else {
>>> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c 
>>> b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
>>> index d7a133c0c5b4..870c5b0d1da7 100644
>>> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
>>> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
>>> @@ -508,9 +508,10 @@ ShellCommandRunConnect (
>>>         Count  = ShellCommandLineGetCount (Package);
>>>         if (Param1 != NULL) {
>>> -        Status  = ShellConvertStringToUint64 (Param1, &Intermediate, 
>>> TRUE, FALSE);
>>> -        Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> -        if (EFI_ERROR (Status)) {
>>> +        Status = ShellConvertStringToUint64 (Param1, &Intermediate, 
>>> TRUE, FALSE);
>>> +        if (!EFI_ERROR (Status)) {
>>> +          Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> +        } else {
>>>             ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param1);
>>>             ShellStatus = SHELL_INVALID_PARAMETER;
>>>           }
>>> @@ -519,9 +520,10 @@ ShellCommandRunConnect (
>>>         }
>>>         if (Param2 != NULL) {
>>> -        Status  = ShellConvertStringToUint64 (Param2, &Intermediate, 
>>> TRUE, FALSE);
>>> -        Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> -        if (EFI_ERROR (Status)) {
>>> +        Status = ShellConvertStringToUint64 (Param2, &Intermediate, 
>>> TRUE, FALSE);
>>> +        if (!EFI_ERROR (Status)) {
>>> +          Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> +        } else {
>>>             ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param2);
>>>             ShellStatus = SHELL_INVALID_PARAMETER;
>>>           }
>>> diff --git 
>>> a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c 
>>> b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
>>> index 009ae5282b27..fd49d1f7ceb4 100644
>>> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
>>> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
>>> @@ -160,12 +160,17 @@ ShellCommandRunDisconnect (
>>>           Param1 = ShellCommandLineGetRawValue (Package, 1);
>>>           Param2 = ShellCommandLineGetRawValue (Package, 2);
>>>           Param3 = ShellCommandLineGetRawValue (Package, 3);
>>> -        ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, 
>>> FALSE);
>>> -        Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate1) : NULL;
>>> -        ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, 
>>> FALSE);
>>> -        Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate2) : NULL;
>>> -        ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, 
>>> FALSE);
>>> -        Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate3) : NULL;
>>> +        if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, 
>>> &Intermediate1, TRUE, FALSE))) {
>>> +          Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate1) : NULL;
>>> +        }
>>> +
>>> +        if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, 
>>> &Intermediate2, TRUE, FALSE))) {
>>> +          Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate2) : NULL;
>>> +        }
>>> +
>>> +        if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, 
>>> &Intermediate3, TRUE, FALSE))) {
>>> +          Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle 
>>> ((UINTN)Intermediate3) : NULL;
>>> +        }
>>>           if ((Param1 != NULL) && (Handle1 == NULL)) {
>>>             ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
>>> (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"disconnect", Param1);
>>> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c 
>>> b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
>>> index c645c9fd6882..8f70d6b6af39 100644
>>> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
>>> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
>>> @@ -438,25 +438,22 @@ ShellCommandRunDrvDiag (
>>>       ControllerHandleStr = ShellCommandLineGetRawValue (Package, 2);
>>>       ChildHandleStr      = ShellCommandLineGetRawValue (Package, 3);
>>> -    if (DriverHandleStr == NULL) {
>>> -      Handle1 = NULL;
>>> -    } else {
>>> -      ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, 
>>> TRUE, FALSE);
>>> +    if ((DriverHandleStr != NULL) && ShellConvertStringToUint64 
>>> (DriverHandleStr, &Intermediate, TRUE, FALSE)) {
>>>         Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> +    } else {
>>> +      Handle1 = NULL;
>>>       }
>>> -    if (ControllerHandleStr == NULL) {
>>> -      Handle2 = NULL;
>>> -    } else {
>>> -      ShellConvertStringToUint64 (ControllerHandleStr, 
>>> &Intermediate, TRUE, FALSE);
>>> +    if ((ControllerHandleStr != NULL) && ShellConvertStringToUint64 
>>> (ControllerHandleStr, &Intermediate, TRUE, FALSE)) {
>>>         Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> +    } else {
>>> +      Handle2 = NULL;
>>>       }
>>> -    if (ChildHandleStr == NULL) {
>>> -      Handle3 = NULL;
>>> -    } else {
>>> -      ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, 
>>> TRUE, FALSE);
>>> +    if ((ChildHandleStr != NULL) && ShellConvertStringToUint64 
>>> (ChildHandleStr, &Intermediate, TRUE, FALSE)) {
>>>         Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
>>> +    } else {
>>> +      Handle3 = NULL;
>>>       }
>>>       Status = DoDiagnostics (
>>
>>
>> 
>>

  reply	other threads:[~2023-03-28 23:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-24 22:30 [PATCH v7 00/12] Enable New CodeQL Queries Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 02/12] BaseTools/PatchCheck.py: Add PCCTS to tab exemption list Michael Kubacki
2023-03-24 23:50   ` [edk2-devel] " Rebecca Cran
2023-03-24 22:30 ` [PATCH v7 03/12] BaseTools/VfrCompile: Fix potential buffer overwrites Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 04/12] CryptoPkg: Fix conditionally uninitialized variable Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 05/12] MdeModulePkg: Fix conditionally uninitialized variables Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 06/12] MdePkg: " Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 07/12] NetworkPkg: " Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 08/12] PcAtChipsetPkg: " Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 09/12] ShellPkg: " Michael Kubacki
2023-03-28 18:49   ` [edk2-devel] " Oliver Smith-Denny
2023-03-28 19:24     ` Michael Kubacki
2023-03-28 23:06       ` Oliver Smith-Denny [this message]
2023-03-24 22:30 ` [PATCH v7 10/12] UefiCpuPkg: " Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 11/12] .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries Michael Kubacki
2023-03-24 22:30 ` [PATCH v7 12/12] .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries Michael Kubacki
2023-03-28 18:51 ` [edk2-devel] [PATCH v7 00/12] Enable New CodeQL Queries Oliver Smith-Denny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4fe27a2b-e892-cc67-7ea7-762089641384@smith-denny.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox