Re: [patch] MdeModulePkg: Fix GCC build failure

["Bi, Dandan" <dandan.bi@intel.com>]

Got it ! I will enhance the logic! Thanks!

Regards,
Dandan

From: Yao, Jiewen
Sent: Friday, November 25, 2016 10:01 AM
To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel@lists.01.org
Cc: Pete Batard <pete@akeo.ie>
Subject: RE: [patch] MdeModulePkg: Fix GCC build failure


Thanks



I am OK on most part.



However, I think we had better do some basic check for below:


>    //
>    // Create break event
>    //
> -  Status = gBS->CreateEvent (
> +  gBS->CreateEvent (
>                    EVT_TIMER | EVT_NOTIFY_SIGNAL,
>                    TPL_CALLBACK,
>                    EbcDebuggerBreakEventFunc,
>                    NULL,
>                    &mDebuggerPrivate.BreakEvent
>                    );
> -  Status = gBS->SetTimer (
> +  gBS->SetTimer (
>                    mDebuggerPrivate.BreakEvent,
>                    TimerPeriodic,
>                    EFI_DEBUG_BREAK_TIMER_INTERVAL
>                    );

If CreateEvent() fails, it does not make sense to call SetTimer().


Also CloseEvent (mDebuggerPrivate.BreakEvent); is called directly later.


So I suggest we add a Status check for CreateEvent. Also for NULL check in CloseEvent

Thank you
Yao Jiewen


> -----Original Message-----
> From: Bi, Dandan
> Sent: Friday, November 25, 2016 9:43 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: [patch] MdeModulePkg: Fix GCC build failure
>
> Remove the variables which are set but not used.
>
> Cc: Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>
> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>
> ---
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c              | 3
> ---
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c | 5
> -----
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbHook.c          |
> 9 ++++-----
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c        | 2
> --
>  4 files changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
> index e0b85c7..adc1c50 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/Edb.c
> @@ -192,11 +192,10 @@ Returns:
>
>  --*/
>  {
>    UINT64   Address;
>    UINTN    Index;
> -  UINT16   OldInstruction;
>    BOOLEAN  IsHitBreakpoint;
>
>    //
>    // Roll back IP for breakpoint instruction (BREAK(3) : 0x0300)
>    //
> @@ -207,11 +206,10 @@ Returns:
>    //
>    IsHitBreakpoint = FALSE;
>    for (Index = 0; (Index < DebuggerPrivate->DebuggerBreakpointCount) &&
> (Index < EFI_DEBUGGER_BREAKPOINT_MAX); Index++) {
>      if
> ((DebuggerPrivate->DebuggerBreakpointContext[Index].BreakpointAddress
> == Address) &&
>          (DebuggerPrivate->DebuggerBreakpointContext[Index].State)) {
> -      OldInstruction =
> (UINT16)DebuggerPrivate->DebuggerBreakpointContext[Index].OldInstructi
> on;
>        IsHitBreakpoint = TRUE;
>        break;
>      }
>    }
>
> @@ -242,11 +240,10 @@ Returns:
>      //
>      IsHitBreakpoint = FALSE;
>      for (Index = 0; (Index < DebuggerPrivate->DebuggerBreakpointCount)
> && (Index < EFI_DEBUGGER_BREAKPOINT_MAX); Index++) {
>        if
> ((DebuggerPrivate->DebuggerBreakpointContext[Index].BreakpointAddress
> == Address) &&
>            (DebuggerPrivate->DebuggerBreakpointContext[Index].State))
> {
> -        OldInstruction =
> (UINT16)DebuggerPrivate->DebuggerBreakpointContext[Index].OldInstructi
> on;
>          IsHitBreakpoint = TRUE;
>          break;
>        }
>      }
>
> diff --git
> a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
> index 4be6dd4..a137ea9 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
> +++
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdBreakpoint.c
> @@ -437,12 +437,10 @@ Returns:
>
>  --*/
>  {
>    UINTN      Index;
>    EFI_STATUS Status;
> -  UINTN      Address;
> -  UINT16     OldInstruction;
>
>    if (CommandArg == NULL) {
>      EDBPrint (L"BreakpointClear Argument error!\n");
>      return EFI_DEBUG_CONTINUE;
>    }
> @@ -464,13 +462,10 @@ Returns:
>
>    if ((Index >= EFI_DEBUGGER_BREAKPOINT_MAX) ||
>        (Index >= DebuggerPrivate->DebuggerBreakpointCount)) {
>      EDBPrint (L"BreakpointClear error!\n");
>      return EFI_DEBUG_CONTINUE;
> -  } else {
> -    Address =
> (UINTN)DebuggerPrivate->DebuggerBreakpointContext[Index].BreakpointA
> ddress;
> -    OldInstruction =
> (UINT16)DebuggerPrivate->DebuggerBreakpointContext[Index].OldInstructi
> on;
>    }
>
>    //
>    // Delete breakpoint
>    //
> diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbHook.c
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbHook.c
> index 9196adb..696ab74 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbHook.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbHook.c
> @@ -387,11 +387,10 @@ Arguments:
>  Returns:
>    None
>
>  --*/
>  {
> -  EFI_STATUS                 Status;
>    UINTN                      Index;
>    EFI_DEBUGGER_SYMBOL_OBJECT *Object;
>    EFI_DEBUGGER_SYMBOL_ENTRY  *Entry;
>
>
> @@ -431,35 +430,35 @@ Returns:
>    }
>
>    //
>    // locate PciRootBridgeIo
>    //
> -  Status = gBS->LocateProtocol (
> +  gBS->LocateProtocol (
>                    &gEfiPciRootBridgeIoProtocolGuid,
>                    NULL,
>                    (VOID**) &mDebuggerPrivate.PciRootBridgeIo
>                    );
>
>    //
>    // locate DebugImageInfoTable
>    //
> -  Status = EfiGetSystemConfigurationTable (
> +  EfiGetSystemConfigurationTable (
>               &gEfiDebugImageInfoTableGuid,
>               (VOID**)
> &mDebuggerPrivate.DebugImageInfoTableHeader
>               );
>
>    //
>    // Create break event
>    //
> -  Status = gBS->CreateEvent (
> +  gBS->CreateEvent (
>                    EVT_TIMER | EVT_NOTIFY_SIGNAL,
>                    TPL_CALLBACK,
>                    EbcDebuggerBreakEventFunc,
>                    NULL,
>                    &mDebuggerPrivate.BreakEvent
>                    );
> -  Status = gBS->SetTimer (
> +  gBS->SetTimer (
>                    mDebuggerPrivate.BreakEvent,
>                    TimerPeriodic,
>                    EFI_DEBUG_BREAK_TIMER_INTERVAL
>                    );
>
> diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
> index 9573c43..a1d21aa 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
> @@ -191,19 +191,17 @@ Returns:
>
>  --*/
>  {
>    CHAR8                      *LineBuffer;
>    CHAR8                      *FieldBuffer;
> -  EFI_DEBUGGER_SYMBOL_ENTRY  *Entry;
>    EDB_EBC_MAP_PARSE_STATE    MapParseState;
>    EDB_EBC_SYMBOL_PARSE_STATE SymbolParseState;
>    CHAR8                      *Name;
>    CHAR8                      *ObjName;
>    UINTN                      Address;
>    EFI_DEBUGGER_SYMBOL_TYPE   Type;
>
> -  Entry = Object->Entry;
>
>    //
>    // Begin to parse the Buffer
>    //
>    LineBuffer = AsciiStrGetNewTokenLine (Buffer, "\n\r");
> --
> 1.9.5.msysgit.1