| Message ID | 0877601216922E4B83A7129715B5DA2BBE7474DF30@GEORGE.Emea.Arm.com |
|---|---|
| State | New |
| Headers | show |
Olivier, For the removing of the Status variable. Is it better to add a check to see if the function passed instead of (or in addition to) removing the variable? -Jaben > -----Original Message----- > From: Olivier Martin [mailto:Olivier.Martin@arm.com] > Sent: Friday, September 19, 2014 3:57 PM > To: Tian, Feng > Cc: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned > variable > > Dear MdeModulePkg maintainer, > please review my attached patch that removes some unused assigned > variables. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > Best Regards, > Olivier > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 ARM Holdings plc, > Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in > England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
Jaben, For this patch, the removed Status variable is truly useless. Two function use the status: For the change in MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c file, the function directly return the value without use the Status parameter. For the change in MdeModulePkg/Universal/SetupBrowserDxe/Setup.c file, browser check the return mFormDisplay pointer instead of check the Status code. Olivier, The patch is good. Reviewed-by: Eric Dong <eric.dong@intel.com> Thanks, Eric -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com] Sent: Saturday, September 20, 2014 7:37 AM To: edk2-devel@lists.sourceforge.net; Tian, Feng Subject: Re: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned variable Olivier, For the removing of the Status variable. Is it better to add a check to see if the function passed instead of (or in addition to) removing the variable? -Jaben > -----Original Message----- > From: Olivier Martin [mailto:Olivier.Martin@arm.com] > Sent: Friday, September 19, 2014 3:57 PM > To: Tian, Feng > Cc: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned > variable > > Dear MdeModulePkg maintainer, > please review my attached patch that removes some unused assigned > variables. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > Best Regards, > Olivier > > -- IMPORTANT NOTICE: The contents of this email and any attachments > are confidential and may also be privileged. If you are not the > intended recipient, please notify the sender immediately and do not > disclose the contents to any other person, use it for any purpose, or > store or copy the information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 ARM Holdings plc, > Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in > England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
Checked in code at R16189. Thanks, Eric -----Original Message----- From: Dong, Eric [mailto:eric.dong@intel.com] Sent: Monday, September 22, 2014 2:23 PM To: edk2-devel@lists.sourceforge.net; Tian, Feng Subject: Re: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned variable Jaben, For this patch, the removed Status variable is truly useless. Two function use the status: For the change in MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c file, the function directly return the value without use the Status parameter. For the change in MdeModulePkg/Universal/SetupBrowserDxe/Setup.c file, browser check the return mFormDisplay pointer instead of check the Status code. Olivier, The patch is good. Reviewed-by: Eric Dong <eric.dong@intel.com> Thanks, Eric -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com] Sent: Saturday, September 20, 2014 7:37 AM To: edk2-devel@lists.sourceforge.net; Tian, Feng Subject: Re: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned variable Olivier, For the removing of the Status variable. Is it better to add a check to see if the function passed instead of (or in addition to) removing the variable? -Jaben > -----Original Message----- > From: Olivier Martin [mailto:Olivier.Martin@arm.com] > Sent: Friday, September 19, 2014 3:57 PM > To: Tian, Feng > Cc: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH] MdeModulePkg/Universal: Fixed unused assigned > variable > > Dear MdeModulePkg maintainer, > please review my attached patch that removes some unused assigned > variables. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > Best Regards, > Olivier > > -- IMPORTANT NOTICE: The contents of this email and any attachments > are confidential and may also be privileged. If you are not the > intended recipient, please notify the sender immediately and do not > disclose the contents to any other person, use it for any purpose, or > store or copy the information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 ARM Holdings plc, > Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in > England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
From 32fffe05a8dee86da2d13c7278ddcec72e027d6c Mon Sep 17 00:00:00 2001 From: Olivier Martin <olivier.martin@arm.com> Date: Fri, 19 Sep 2014 22:18:53 +0100 Subject: MdeModulePkg/Universal: Fixed unused assigned variable Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <olivier.martin@arm.com> --- .../CustomizedDisplayLib/CustomizedDisplayLib.c | 2 -- .../Universal/DisplayEngineDxe/FormDisplay.c | 10 ---------- .../Universal/DisplayEngineDxe/InputHandler.c | 8 ++------ .../Universal/DisplayEngineDxe/ProcessOptions.c | 2 -- MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 9 +-------- 5 files changed, 3 insertions(+), 28 deletions(-) diff --git a/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c b/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c index 92f3a43..f442f7d 100644 --- a/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c +++ b/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c @@ -135,7 +135,6 @@ RefreshKeyHelp ( { UINTN SecCol; UINTN ThdCol; - UINTN LeftColumnOfHelp; UINTN RightColumnOfHelp; UINTN TopRowOfHelp; UINTN BottomRowOfHelp; @@ -166,7 +165,6 @@ RefreshKeyHelp ( ThdCol = gScreenDimensions.LeftColumn + (gScreenDimensions.RightColumn - gScreenDimensions.LeftColumn) / 3 * 2; StartColumnOfHelp = gScreenDimensions.LeftColumn + 2; - LeftColumnOfHelp = gScreenDimensions.LeftColumn + 1; RightColumnOfHelp = gScreenDimensions.RightColumn - 1; TopRowOfHelp = gScreenDimensions.BottomRow - STATUS_BAR_HEIGHT - gFooterHeight + 1; BottomRowOfHelp = gScreenDimensions.BottomRow - STATUS_BAR_HEIGHT - 2; diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c index 0db450e..4be8a96 100644 --- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c +++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c @@ -2049,10 +2049,8 @@ UiDisplayMenu ( UINTN TopRow; UINTN BottomRow; UINTN Index; - UINT16 Width; CHAR16 *StringPtr; CHAR16 *OptionString; - CHAR16 *OutputString; CHAR16 *HelpString; CHAR16 *HelpHeaderString; CHAR16 *HelpBottomString; @@ -2069,10 +2067,8 @@ UiDisplayMenu ( UI_MENU_OPTION *MenuOption; UI_MENU_OPTION *NextMenuOption; UI_MENU_OPTION *SavedMenuOption; - UI_MENU_OPTION *PreviousMenuOption; UI_CONTROL_FLAG ControlFlag; UI_SCREEN_OPERATION ScreenOperation; - UINT16 DefaultId; FORM_DISPLAY_ENGINE_STATEMENT *Statement; BROWSER_HOT_KEY *HotKey; UINTN HelpPageIndex; @@ -2087,7 +2083,6 @@ UiDisplayMenu ( UINT16 BottomLineWidth; EFI_STRING_ID HelpInfo; UI_EVENT_TYPE EventType; - FORM_DISPLAY_ENGINE_STATEMENT *InitialHighlight; BOOLEAN SkipHighLight; EventType = UIEventNone; @@ -2098,7 +2093,6 @@ UiDisplayMenu ( OptionString = NULL; ScreenOperation = UiNoOperation; NewLine = TRUE; - DefaultId = 0; HelpPageCount = 0; HelpLine = 0; RowCount = 0; @@ -2109,24 +2103,20 @@ UiDisplayMenu ( EachLineWidth = 0; HeaderLineWidth = 0; BottomLineWidth = 0; - OutputString = NULL; UpArrow = FALSE; DownArrow = FALSE; SkipValue = 0; SkipHighLight = FALSE; NextMenuOption = NULL; - PreviousMenuOption = NULL; SavedMenuOption = NULL; HotKey = NULL; Repaint = TRUE; MenuOption = NULL; gModalSkipColumn = (CHAR16) (gStatementDimensions.RightColumn - gStatementDimensions.LeftColumn) / 6; - InitialHighlight = gFormData->HighLightedStatement; ZeroMem (&Key, sizeof (EFI_INPUT_KEY)); - Width = (UINT16)gOptionBlockWidth - 1; TopRow = gStatementDimensions.TopRow + SCROLL_ARROW_HEIGHT; BottomRow = gStatementDimensions.BottomRow - SCROLL_ARROW_HEIGHT - 1; diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c b/MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c index 78dd104..f76937a 100644 --- a/MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c +++ b/MdeModulePkg/Universal/DisplayEngineDxe/InputHandler.c @@ -439,7 +439,6 @@ GetNumericInput ( IN UI_MENU_OPTION *MenuOption ) { - EFI_STATUS Status; UINTN Column; UINTN Row; CHAR16 InputText[MAX_NUMERIC_INPUT_WIDTH]; @@ -685,7 +684,7 @@ GetNumericInput ( goto TheKey2; } - Status = WaitForKeyStroke (&Key); + WaitForKeyStroke (&Key); TheKey2: switch (Key.UnicodeChar) { @@ -1118,7 +1117,6 @@ GetSelectionInputPopUp ( IN UI_MENU_OPTION *MenuOption ) { - EFI_STATUS Status; EFI_INPUT_KEY Key; UINTN Index; CHAR16 *StringPtr; @@ -1351,7 +1349,7 @@ GetSelectionInputPopUp ( goto TheKey; } - Status = WaitForKeyStroke (&Key); + WaitForKeyStroke (&Key); TheKey: switch (Key.UnicodeChar) { @@ -1505,7 +1503,6 @@ TheKey: } else { gUserInput->InputValue.Buffer = ReturnValue; gUserInput->InputValue.BufferLen = Question->CurrentValue.BufferLen; - Status = EFI_SUCCESS; } } else { ASSERT (CurrentOption != NULL); @@ -1514,7 +1511,6 @@ TheKey: return EFI_DEVICE_ERROR; } else { SetValuesByType (&gUserInput->InputValue.Value, &CurrentOption->OptionOpCode->Value, gUserInput->InputValue.Type); - Status = EFI_SUCCESS; } } diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c index 65d2a74..6e9e6dd 100644 --- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c +++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c @@ -955,7 +955,6 @@ ProcessOptions ( UINTN Index2; UINT8 *ValueArray; UINT8 ValueType; - EFI_STRING_ID StringId; EFI_IFR_ORDERED_LIST *OrderList; BOOLEAN ValueInvalid; @@ -964,7 +963,6 @@ ProcessOptions ( StringPtr = NULL; Character[1] = L'\0'; *OptionString = NULL; - StringId = 0; ValueInvalid = FALSE; ZeroMem (FormattedNumber, 21 * sizeof (CHAR16)); diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c index 2b09593..b4fd878 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c @@ -836,13 +836,11 @@ FormDisplayCallback ( IN VOID *Context ) { - EFI_STATUS Status; - if (mFormDisplay != NULL) { return; } - Status = gBS->LocateProtocol ( + gBS->LocateProtocol ( &gEdkiiFormDisplayEngineProtocolGuid, NULL, (VOID **) &mFormDisplay @@ -3205,7 +3203,6 @@ SubmitForSystem ( EFI_STATUS Status; LIST_ENTRY *Link; LIST_ENTRY *StorageLink; - BROWSER_STORAGE *Storage; FORMSET_STORAGE *FormSetStorage; FORM_BROWSER_FORM *Form; FORM_BROWSER_FORMSET *LocalFormSet; @@ -3271,7 +3268,6 @@ SubmitForSystem ( StorageLink = GetFirstNode (&LocalFormSet->StorageListHead); while (!IsNull (&LocalFormSet->StorageListHead, StorageLink)) { FormSetStorage = FORMSET_STORAGE_FROM_LINK (StorageLink); - Storage = FormSetStorage->BrowserStorage; StorageLink = GetNextNode (&LocalFormSet->StorageListHead, StorageLink); SynchronizeStorage(FormSetStorage->BrowserStorage, FormSetStorage->ConfigRequest, FALSE); @@ -3280,7 +3276,6 @@ SubmitForSystem ( StorageLink = GetFirstNode (&LocalFormSet->SaveFailStorageListHead); while (!IsNull (&LocalFormSet->SaveFailStorageListHead, StorageLink)) { FormSetStorage = FORMSET_STORAGE_FROM_SAVE_FAIL_LINK (StorageLink); - Storage = FormSetStorage->BrowserStorage; StorageLink = GetNextNode (&LocalFormSet->SaveFailStorageListHead, StorageLink); SynchronizeStorage(FormSetStorage->BrowserStorage, FormSetStorage->ConfigRequest, FALSE); @@ -4601,13 +4596,11 @@ ConfigRequestAdjust ( CHAR16 *RequestElement; CHAR16 *NextRequestElement; CHAR16 *NextElementBakup; - UINTN SpareBufLen; CHAR16 *SearchKey; CHAR16 *ValueKey; BOOLEAN RetVal; CHAR16 *ConfigRequest; - SpareBufLen = 0; RetVal = FALSE; NextElementBakup = NULL; ValueKey = NULL; -- 1.7.5.4
Dear MdeModulePkg maintainer, please review my attached patch that removes some unused assigned variables. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <olivier.martin@arm.com> Best Regards, Olivier -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk