From 13ad1def34bfd3f8fbfa4bc0bf814518a9047b27 Mon Sep 17 00:00:00 2001 From: lgao4 Date: Fri, 4 Jun 2010 01:29:03 +0000 Subject: [PATCH] Do the following fix up in SetupBrowser driver: 1) Check whether ConfigAccess is NULL before use it. 2) Don't do call back for UI_ACTION_REFRESH_FORMSET action. 3) Release resource before leave SetupBrowser() function. 4) Use the unified check method (HiiHandle, FormsetGuid and FormId) to check FORM is open or close. git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@10565 6f19259b-4bc3-4df7-8a09-765794883524 --- .../Universal/SetupBrowserDxe/Presentation.c | 93 +++++++++---------- .../SetupBrowserDxe/ProcessOptions.c | 3 +- .../Universal/SetupBrowserDxe/Setup.c | 12 +-- 3 files changed, 51 insertions(+), 57 deletions(-) diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c index e9b4df4df9..a2704511c2 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c @@ -16,7 +16,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. BOOLEAN mHiiPackageListUpdated; UI_MENU_SELECTION *gCurrentSelection; - +EFI_HII_HANDLE mCurrentHiiHandle = NULL; +EFI_GUID mCurrentFormSetGuid = {0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0}}; +UINT16 mCurrentFormId = 0; /** Clear retangle with specified text attribute. @@ -842,8 +844,6 @@ FormUpdateNotify ( return EFI_SUCCESS; } -BOOLEAN mFormCloseCallBack = FALSE; - /** The worker function that send the displays to the screen. On output, the selection made by user is returned. @@ -872,11 +872,7 @@ SetupBrowser ( EFI_HII_CONFIG_ACCESS_PROTOCOL *ConfigAccess; FORM_BROWSER_FORMSET *FormSet; EFI_INPUT_KEY Key; - BOOLEAN FormOpenCallBack; BOOLEAN SubmitFormIsRequired; - EFI_GUID CurrentFormSetGuid; - EFI_HII_HANDLE CurrentHiiHandle; - UINT16 CurrentFormId; gMenuRefreshHead = NULL; gResetRequired = FALSE; @@ -903,7 +899,6 @@ SetupBrowser ( // Status = InitializeCurrentSetting (Selection->FormSet); if (EFI_ERROR (Status)) { - Selection->Action = UI_ACTION_EXIT; goto Done; } @@ -911,13 +906,7 @@ SetupBrowser ( // // Initialize Selection->Form // - FormOpenCallBack = FALSE; if (Selection->FormId == 0) { - // - // First Form will open. - // - FormOpenCallBack = TRUE; - // // Zero FormId indicates display the first Form in a FormSet // @@ -926,12 +915,6 @@ SetupBrowser ( Selection->Form = FORM_BROWSER_FORM_FROM_LINK (Link); Selection->FormId = Selection->Form->FormId; } else { - if (Selection->Form == NULL) { - // - // First Form will open. - // - FormOpenCallBack = TRUE; - } Selection->Form = IdToForm (Selection->FormSet, Selection->FormId); } @@ -939,7 +922,8 @@ SetupBrowser ( // // No Form to display // - return EFI_NOT_FOUND; + Status = EFI_NOT_FOUND; + goto Done; } // @@ -947,8 +931,9 @@ SetupBrowser ( // if (Selection->Form->SuppressExpression != NULL) { Status = EvaluateExpression (Selection->FormSet, Selection->Form, Selection->Form->SuppressExpression); - if (EFI_ERROR (Status)) { - return Status; + if (EFI_ERROR (Status) || (Selection->Form->SuppressExpression->Result.Type != EFI_IFR_TYPE_BOOLEAN)) { + Status = EFI_INVALID_PARAMETER; + goto Done; } if (Selection->Form->SuppressExpression->Result.Value.b) { @@ -959,17 +944,11 @@ SetupBrowser ( CreateDialog (4, TRUE, 0, NULL, &Key, gEmptyString, gFormSuppress, gPressEnter, gEmptyString); } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN); - return EFI_NOT_FOUND; + Status = EFI_NOT_FOUND; + goto Done; } } - // - // Keep current form information - // - CurrentHiiHandle = Selection->Handle; - CopyGuid (&CurrentFormSetGuid, &Selection->FormSetGuid); - CurrentFormId = Selection->FormId; - // // Reset FormPackage update flag // @@ -980,8 +959,18 @@ SetupBrowser ( // for each question with callback flag. // New form may be the first form, or the different form after another form close. // - if ((FormOpenCallBack || mFormCloseCallBack) && (ConfigAccess != NULL)) { - mFormCloseCallBack = FALSE; + if ((ConfigAccess != NULL) && + ((Selection->Handle != mCurrentHiiHandle) || + (!CompareGuid (&Selection->FormSetGuid, &mCurrentFormSetGuid)) || + (Selection->FormId != mCurrentFormId))) { + + // + // Keep current form information + // + mCurrentHiiHandle = Selection->Handle; + CopyGuid (&mCurrentFormSetGuid, &Selection->FormSetGuid); + mCurrentFormId = Selection->FormId; + // // Go through each statement in this form // @@ -1061,7 +1050,7 @@ SetupBrowser ( // Status = LoadFormSetConfig (Selection, Selection->FormSet); if (EFI_ERROR (Status)) { - return Status; + goto Done; } // @@ -1089,7 +1078,7 @@ SetupBrowser ( // Status = DisplayForm (Selection); if (EFI_ERROR (Status)) { - return Status; + goto Done; } // @@ -1106,12 +1095,11 @@ SetupBrowser ( // mHiiPackageListUpdated = FALSE; - if (((Statement->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK) && (Statement->Operand != EFI_IFR_PASSWORD_OP)) { - ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE; + if ((ConfigAccess != NULL) && + ((Statement->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK) && + (Statement->Operand != EFI_IFR_PASSWORD_OP)) { - if (ConfigAccess == NULL) { - return EFI_UNSUPPORTED; - } + ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE; HiiValue = &Statement->HiiValue; TypeValue = &HiiValue->Value; @@ -1191,16 +1179,14 @@ SetupBrowser ( // Before exit the form, invoke ConfigAccess.Callback() with EFI_BROWSER_ACTION_FORM_CLOSE // for each question with callback flag. // - mFormCloseCallBack = FALSE; if ((ConfigAccess != NULL) && ((Selection->Action == UI_ACTION_EXIT) || - (Selection->Handle != CurrentHiiHandle) || - (!CompareGuid (&CurrentFormSetGuid, &Selection->FormSetGuid)) || - (Selection->FormId != CurrentFormId))) { + (Selection->Handle != mCurrentHiiHandle) || + (!CompareGuid (&Selection->FormSetGuid, &mCurrentFormSetGuid)) || + (Selection->FormId != mCurrentFormId))) { // // Go through each statement in this form // - mFormCloseCallBack = TRUE; SubmitFormIsRequired = FALSE; Link = GetFirstNode (&Selection->Form->StatementListHead); while (!IsNull (&Selection->Form->StatementListHead, Link)) { @@ -1256,12 +1242,21 @@ SetupBrowser ( gOldFormSet = FormSet; Done: + // + // Reset current form information to the initial setting when error happens or form exit. + // + if (EFI_ERROR (Status) || Selection->Action == UI_ACTION_EXIT) { + mCurrentHiiHandle = NULL; + CopyGuid (&mCurrentFormSetGuid, &gZeroGuid); + mCurrentFormId = 0; + } + // // Unregister notify for Form package update // - Status = mHiiDatabase->UnregisterPackageNotify ( - mHiiDatabase, - NotifyHandle - ); + mHiiDatabase->UnregisterPackageNotify ( + mHiiDatabase, + NotifyHandle + ); return Status; } diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c index 6bd2063caf..a4d19e0afe 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c @@ -778,8 +778,9 @@ ProcessOptions ( // *StringPtr = 0; Status = PasswordCallback (Selection, MenuOption, StringPtr); - if (Status == EFI_NOT_AVAILABLE_YET) { + if (Status == EFI_NOT_AVAILABLE_YET || Status == EFI_UNSUPPORTED) { // + // Callback is not supported, or // Callback request to terminate password input // FreePool (StringPtr); diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c index e06228d4aa..42edc08970 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c @@ -2080,15 +2080,13 @@ LoadFormConfig ( // // Check whether EfiVarstore with CallBack can be got. // - if ((Question->QuestionId != 0) && (Question->Storage != NULL) && + if ((FormSet->ConfigAccess != NULL) && + (Selection->Action != UI_ACTION_REFRESH_FORMSET) && + (Question->QuestionId != 0) && + (Question->Storage != NULL) && (Question->Storage->Type == EFI_HII_VARSTORE_EFI_VARIABLE) && ((Question->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK)) { - // - // ConfigAccess can't be NULL. - // - if (FormSet->ConfigAccess == NULL) { - return EFI_UNSUPPORTED; - } + // // Check QuestionValue does exist. //