Reviewed-by: Liming Gao <liming....@intel.com> -----Original Message----- From: Bi, Dandan Sent: Monday, January 25, 2016 8:58 AM To: edk2-devel@lists.01.org Cc: Gao, Liming; Dong, Eric Subject: [patch] MdeModulePkg:Make the logic in ConfigRouting.c clear and safe
The BlockData is expected to be NULL when to call function IsThisOpcodeRequired in each opcode,but now exists case that the Blockdata not be cleaned,then will be used in other opcode.it is not correct,now add the check before use. The comments and logic in function IsThisOpcodeRequired are not consistent,now refine the code to make the logic clear. Cc: Liming Gao <liming....@intel.com> Cc: Eric Dong <eric.d...@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Dandan Bi <dandan...@intel.com> Reviewed-by: Eric Dong <eric.d...@intel.com> --- .../Universal/HiiDatabaseDxe/ConfigRouting.c | 145 ++++++++++++++++++--- 1 file changed, 128 insertions(+), 17 deletions(-) diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c index 8f0b968..5b66b97 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c @@ -1,9 +1,9 @@ /** @file Implementation of interfaces function for EFI_HII_CONFIG_ROUTING_PROTOCOL. -Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at http://opensource.org/licenses/bsd-license.php @@ -1464,11 +1464,12 @@ Done: @param IfrOpHdr Ifr opcode header for this opcode. @param VarWidth The buffer width for this opcode. @param ReturnData The data block added for this opcode. @retval EFI_SUCCESS This opcode is required. - @retval Others This opcode is not required or error occur. + @retval EFI_NOT_FOUND This opcode is not required. + @retval Others Contain some error. **/ EFI_STATUS IsThisOpcodeRequired ( IN IFR_BLOCK_DATA *RequestBlockArray, @@ -1496,11 +1497,11 @@ IsThisOpcodeRequired ( // if (!BlockArrayCheck (RequestBlockArray, NameId, 0, TRUE, HiiHandle)) { // // This question is not in the requested string. Skip it. // - return EFI_SUCCESS; + return EFI_NOT_FOUND; } } else { VarOffset = IfrQuestionHdr->VarStoreInfo.VarOffset; // @@ -1508,11 +1509,11 @@ IsThisOpcodeRequired ( // if (!BlockArrayCheck (RequestBlockArray, VarOffset, VarWidth, FALSE, HiiHandle)) { // // This question is not in the requested string. Skip it. // - return EFI_SUCCESS; + return EFI_NOT_FOUND; } // // Check this var question is in the var storage // @@ -1770,12 +1771,25 @@ ParseIfrData ( if (IfrRef->Question.VarStoreId != VarStoreId) { break; } VarWidth = (UINT16) (sizeof (EFI_HII_REF)); + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } break; case EFI_IFR_ONE_OF_OP: @@ -1798,21 +1812,32 @@ ParseIfrData ( if (IfrOneOf->Question.VarStoreId != VarStoreId) { break; } VarWidth = (UINT16) (1 << (IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE)); + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } - - if (BlockData == NULL) { - // - // BlockData == NULL means this opcode is not in the requst array. - // - break; - } + + // + //when go to there,BlockData can't be NULLL. + // + ASSERT (BlockData != NULL) ; if (IfrOpHdr->OpCode == EFI_IFR_ONE_OF_OP) { // // Set this flag to TRUE for the first oneof option. // @@ -1875,12 +1900,26 @@ ParseIfrData ( if (IfrOrderedList->Question.VarStoreId != VarStoreId) { BlockData = NULL; break; } VarWidth = IfrOrderedList->MaxContainers; + + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } break; case EFI_IFR_CHECKBOX_OP: @@ -1906,21 +1945,33 @@ ParseIfrData ( IfrCheckBox = (EFI_IFR_CHECKBOX *) IfrOpHdr; if (IfrCheckBox->Question.VarStoreId != VarStoreId) { break; } VarWidth = (UINT16) sizeof (BOOLEAN); + + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } - if (BlockData == NULL) { - // - // BlockData == NULL means this opcode is not in the requst array. - // - break; - } + // + //when go to there,BlockData can't be NULLL. + // + ASSERT (BlockData != NULL) ; // // Add default value for standard ID by CheckBox Flag // VarDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD; @@ -1993,13 +2044,26 @@ ParseIfrData ( IfrDate = (EFI_IFR_DATE *) IfrOpHdr; if (IfrDate->Question.VarStoreId != VarStoreId) { break; } + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + VarWidth = (UINT16) sizeof (EFI_HII_DATE); Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } break; case EFI_IFR_TIME_OP: @@ -2022,13 +2086,26 @@ ParseIfrData ( IfrTime = (EFI_IFR_TIME *) IfrOpHdr; if (IfrTime->Question.VarStoreId != VarStoreId) { break; } + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + VarWidth = (UINT16) sizeof (EFI_HII_TIME); Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } break; case EFI_IFR_STRING_OP: @@ -2051,13 +2128,26 @@ ParseIfrData ( IfrString = (EFI_IFR_STRING *) IfrOpHdr; if (IfrString->Question.VarStoreId != VarStoreId) { break; } + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + VarWidth = (UINT16) (IfrString->MaxSize * sizeof (UINT16)); Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } break; case EFI_IFR_PASSWORD_OP: @@ -2080,13 +2170,26 @@ ParseIfrData ( IfrPassword = (EFI_IFR_PASSWORD *) IfrOpHdr; if (IfrPassword->Question.VarStoreId != VarStoreId) { break; } + // + // The BlockData may allocate by other opcode,need to clean. + // + if (BlockData != NULL){ + BlockData = NULL; + } + VarWidth = (UINT16) (IfrPassword->MaxSize * sizeof (UINT16)); Status = IsThisOpcodeRequired(RequestBlockArray, HiiHandle, VarStorageData, IfrOpHdr, VarWidth, &BlockData); if (EFI_ERROR (Status)) { + if (Status == EFI_NOT_FOUND){ + // + //The opcode is not required,exit and parse other opcode. + // + break; + } goto Done; } // // No default value for string. @@ -2286,10 +2389,18 @@ ParseIfrData ( IfrOffset += IfrOpHdr->Length; PackageOffset += IfrOpHdr->Length; } + // + //if Status == EFI_NOT_FOUND, just means the opcode is not + required,not contain any error, //so set the Status to EFI_SUCCESS. + // + if (Status == EFI_NOT_FOUND){ + Status = EFI_SUCCESS; + } + Done: for (LinkData = VarStorageData->BlockEntry.ForwardLink; LinkData != &VarStorageData->BlockEntry; LinkData = LinkData->ForwardLink) { BlockData = BASE_CR (LinkData, IFR_BLOCK_DATA, Entry); for (LinkDefault = BlockData->DefaultValueEntry.ForwardLink; LinkDefault != &BlockData->DefaultValueEntry; ) { DefaultDataPtr = BASE_CR (LinkDefault, IFR_DEFAULT_DATA, Entry); -- 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
