public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] MdeModulePkg/CapsuleLib: Code refinements
@ 2017-03-10  6:06 Hao Wu
  2017-03-10  6:06 ` [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events Hao Wu
  2017-03-10  6:07 ` [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API Hao Wu
  0 siblings, 2 replies; 6+ messages in thread
From: Hao Wu @ 2017-03-10  6:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jiewen Yao

The series makes the following refinements in DxeCapsuleLibFmp:
1) Add library destructors to close the events created in their relative
constructors.

2) Free the buffer returned by the GetVariable2 API.

Cc: Jiewen Yao <jiewen.yao@intel.com>

Hao Wu (2):
  MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events
  MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c          | 37 +++++++++++++++++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c    |  3 ++
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c      | 33 ++++++++++++++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 5 files changed, 69 insertions(+), 11 deletions(-)

-- 
2.12.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events
  2017-03-10  6:06 [PATCH 0/2] MdeModulePkg/CapsuleLib: Code refinements Hao Wu
@ 2017-03-10  6:06 ` Hao Wu
  2017-03-10  6:13   ` Yao, Jiewen
  2017-03-10  6:07 ` [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API Hao Wu
  1 sibling, 1 reply; 6+ messages in thread
From: Hao Wu @ 2017-03-10  6:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jiewen Yao

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c          | 37 +++++++++++++++++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c      | 33 ++++++++++++++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 4 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 7f500a96eb..a892892ccd 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -47,9 +47,11 @@
 #include <Protocol/FirmwareManagement.h>
 #include <Protocol/DevicePath.h>
 
-EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable              = NULL;
-BOOLEAN                   mIsVirtualAddrConverted  = FALSE;
-BOOLEAN                   mDxeCapsuleLibEndOfDxe   = FALSE;
+EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable                                     = NULL;
+BOOLEAN                   mIsVirtualAddrConverted                         = FALSE;
+BOOLEAN                   mDxeCapsuleLibEndOfDxe                          = FALSE;
+EFI_EVENT                 mDxeCapsuleLibEndOfDxeEvent                     = NULL;
+EFI_EVENT                 mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
 
 /**
   Initialize capsule related variables.
@@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
-  EFI_EVENT     EndOfDxeEvent;
   EFI_STATUS    Status;
 
   Status = gBS->CreateEventEx (
@@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
                   DxeCapsuleLibEndOfDxe,
                   NULL,
                   &gEfiEndOfDxeEventGroupGuid,
-                  &EndOfDxeEvent
+                  &mDxeCapsuleLibEndOfDxeEvent
                   );
   ASSERT_EFI_ERROR (Status);
 
@@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
 
   return EFI_SUCCESS;
 }
+
+/**
+  The destructor function closes the End of DXE event.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The destructor completed successfully.
+**/
+EFI_STATUS
+EFIAPI
+DxeCapsuleLibDestructor (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS    Status;
+
+  //
+  // Close the End of DXE event.
+  //
+  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
index 5e437dc418..a6cf54cb6b 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_DRIVER module types.
 #
-#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 2017, 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
@@ -23,6 +23,7 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = CapsuleLib|DXE_DRIVER UEFI_APPLICATION
   CONSTRUCTOR                    = DxeCapsuleLibConstructor
+  DESTRUCTOR                     = DxeCapsuleLibDestructor
 
 #
 # The following information is for reference only and not required by the build tools.
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
index 880143905a..513aa533e1 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
@@ -1,7 +1,7 @@
 /** @file
   Capsule library runtime support.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2017, 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
@@ -28,6 +28,7 @@
 
 extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
 extern BOOLEAN                   mIsVirtualAddrConverted;
+extern EFI_EVENT                 mDxeRuntimeCapsuleLibVirtualAddressChangeEvent;
 
 /**
   Convert EsrtTable physical address to virtual address.
@@ -92,21 +93,45 @@ DxeRuntimeCapsuleLibConstructor (
   )
 {
   EFI_STATUS     Status;
-  EFI_EVENT      Event;
 
   //
   // Make sure we can handle virtual address changes.
   //
-  Event = NULL;
   Status = gBS->CreateEventEx (
                   EVT_NOTIFY_SIGNAL,
                   TPL_NOTIFY,
                   DxeCapsuleLibVirtualAddressChangeEvent,
                   NULL,
                   &gEfiEventVirtualAddressChangeGuid,
-                  &Event
+                  &mDxeRuntimeCapsuleLibVirtualAddressChangeEvent
                   );
   ASSERT_EFI_ERROR (Status);
 
   return EFI_SUCCESS;
 }
+
+/**
+  The destructor function closes the VirtualAddressChange event.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The destructor completed successfully.
+**/
+EFI_STATUS
+EFIAPI
+DxeRuntimeCapsuleLibDestructor (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS    Status;
+
+  //
+  // Close the VirtualAddressChange event.
+  //
+  Status = gBS->CloseEvent (mDxeRuntimeCapsuleLibVirtualAddressChangeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
index 47c67ccac5..25b7d51f57 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_RUNTIME_DRIVER module types.
 #
-#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 2017, 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
@@ -24,6 +24,8 @@
   LIBRARY_CLASS                  = CapsuleLib|DXE_RUNTIME_DRIVER
   CONSTRUCTOR                    = DxeCapsuleLibConstructor
   CONSTRUCTOR                    = DxeRuntimeCapsuleLibConstructor
+  DESTRUCTOR                     = DxeCapsuleLibDestructor
+  DESTRUCTOR                     = DxeRuntimeCapsuleLibDestructor
 
 #
 # The following information is for reference only and not required by the build tools.
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API
  2017-03-10  6:06 [PATCH 0/2] MdeModulePkg/CapsuleLib: Code refinements Hao Wu
  2017-03-10  6:06 ` [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events Hao Wu
@ 2017-03-10  6:07 ` Hao Wu
  2017-03-10  6:16   ` Yao, Jiewen
  1 sibling, 1 reply; 6+ messages in thread
From: Hao Wu @ 2017-03-10  6:07 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jiewen Yao

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
index fc0f8698a9..191a432369 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
@@ -343,6 +343,9 @@ InitCapsuleLastVariable (
                         NULL
                         );
       }
+      if (CapsuleResult != NULL) {
+        FreePool (CapsuleResult);
+      }
     }
 
     // Lock it in normal boot path per UEFI spec.
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events
  2017-03-10  6:06 ` [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events Hao Wu
@ 2017-03-10  6:13   ` Yao, Jiewen
  0 siblings, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2017-03-10  6:13 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org

Reviewed-by: Jiewen.yao@Intel.com

> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle
> unclosed events
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c          | 37
> +++++++++++++++++---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  3 +-
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c      | 33
> ++++++++++++++---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> ++-
>  4 files changed, 66 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 7f500a96eb..a892892ccd 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -47,9 +47,11 @@
>  #include <Protocol/FirmwareManagement.h>
>  #include <Protocol/DevicePath.h>
> 
> -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable              = NULL;
> -BOOLEAN                   mIsVirtualAddrConverted  = FALSE;
> -BOOLEAN                   mDxeCapsuleLibEndOfDxe   = FALSE;
> +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> = NULL;
> +BOOLEAN                   mIsVirtualAddrConverted
> = FALSE;
> +BOOLEAN                   mDxeCapsuleLibEndOfDxe
> = FALSE;
> +EFI_EVENT                 mDxeCapsuleLibEndOfDxeEvent
> = NULL;
> +EFI_EVENT
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> 
>  /**
>    Initialize capsule related variables.
> @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
>    IN EFI_SYSTEM_TABLE   *SystemTable
>    )
>  {
> -  EFI_EVENT     EndOfDxeEvent;
>    EFI_STATUS    Status;
> 
>    Status = gBS->CreateEventEx (
> @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
>                    DxeCapsuleLibEndOfDxe,
>                    NULL,
>                    &gEfiEndOfDxeEventGroupGuid,
> -                  &EndOfDxeEvent
> +                  &mDxeCapsuleLibEndOfDxeEvent
>                    );
>    ASSERT_EFI_ERROR (Status);
> 
> @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> 
>    return EFI_SUCCESS;
>  }
> +
> +/**
> +  The destructor function closes the End of DXE event.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The destructor completed successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeCapsuleLibDestructor (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  //
> +  // Close the End of DXE event.
> +  //
> +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 5e437dc418..a6cf54cb6b 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  Capsule library instance for DXE_DRIVER module types.
>  #
> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2016 - 2017, 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
> @@ -23,6 +23,7 @@
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = CapsuleLib|DXE_DRIVER
> UEFI_APPLICATION
>    CONSTRUCTOR                    = DxeCapsuleLibConstructor
> +  DESTRUCTOR                     = DxeCapsuleLibDestructor
> 
>  #
>  # The following information is for reference only and not required by the build
> tools.
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> index 880143905a..513aa533e1 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Capsule library runtime support.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2017, 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
> @@ -28,6 +28,7 @@
> 
>  extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
>  extern BOOLEAN                   mIsVirtualAddrConverted;
> +extern EFI_EVENT
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent;
> 
>  /**
>    Convert EsrtTable physical address to virtual address.
> @@ -92,21 +93,45 @@ DxeRuntimeCapsuleLibConstructor (
>    )
>  {
>    EFI_STATUS     Status;
> -  EFI_EVENT      Event;
> 
>    //
>    // Make sure we can handle virtual address changes.
>    //
> -  Event = NULL;
>    Status = gBS->CreateEventEx (
>                    EVT_NOTIFY_SIGNAL,
>                    TPL_NOTIFY,
>                    DxeCapsuleLibVirtualAddressChangeEvent,
>                    NULL,
>                    &gEfiEventVirtualAddressChangeGuid,
> -                  &Event
> +                  &mDxeRuntimeCapsuleLibVirtualAddressChangeEvent
>                    );
>    ASSERT_EFI_ERROR (Status);
> 
>    return EFI_SUCCESS;
>  }
> +
> +/**
> +  The destructor function closes the VirtualAddressChange event.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The destructor completed successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeCapsuleLibDestructor (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  //
> +  // Close the VirtualAddressChange event.
> +  //
> +  Status = gBS->CloseEvent
> (mDxeRuntimeCapsuleLibVirtualAddressChangeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> index 47c67ccac5..25b7d51f57 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  Capsule library instance for DXE_RUNTIME_DRIVER module types.
>  #
> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2016 - 2017, 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
> @@ -24,6 +24,8 @@
>    LIBRARY_CLASS                  = CapsuleLib|DXE_RUNTIME_DRIVER
>    CONSTRUCTOR                    = DxeCapsuleLibConstructor
>    CONSTRUCTOR                    = DxeRuntimeCapsuleLibConstructor
> +  DESTRUCTOR                     = DxeCapsuleLibDestructor
> +  DESTRUCTOR                     = DxeRuntimeCapsuleLibDestructor
> 
>  #
>  # The following information is for reference only and not required by the build
> tools.
> --
> 2.12.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API
  2017-03-10  6:07 ` [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API Hao Wu
@ 2017-03-10  6:16   ` Yao, Jiewen
  2017-03-10  7:02     ` Wu, Hao A
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2017-03-10  6:16 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org

According to GetVariable2() API, there is no guarantee that Data will be NULL, if error is returned.

I suggest we use status check instead of data pointer check.


/**
  Returns the status whether get the variable success. The function retrieves 
  variable  through the UEFI Runtime Service GetVariable().  The 
  returned buffer is allocated using AllocatePool().  The caller is responsible
  for freeing this buffer with FreePool().

  If Name  is NULL, then ASSERT().
  If Guid  is NULL, then ASSERT().
  If Value is NULL, then ASSERT().

  @param[in]  Name  The pointer to a Null-terminated Unicode string.
  @param[in]  Guid  The pointer to an EFI_GUID structure
  @param[out] Value The buffer point saved the variable info.
  @param[out] Size  The buffer size of the variable.

  @return EFI_OUT_OF_RESOURCES      Allocate buffer failed.
  @return EFI_SUCCESS               Find the specified variable.
  @return Others Errors             Return errors from call to gRT->GetVariable.

**/



> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> GetVariable2 API
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index fc0f8698a9..191a432369 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
>                          NULL
>                          );
>        }
> +      if (CapsuleResult != NULL) {
> +        FreePool (CapsuleResult);
> +      }
>      }
> 
>      // Lock it in normal boot path per UEFI spec.
> --
> 2.12.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API
  2017-03-10  6:16   ` Yao, Jiewen
@ 2017-03-10  7:02     ` Wu, Hao A
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2017-03-10  7:02 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Friday, March 10, 2017 2:17 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: RE: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned
> by GetVariable2 API
> 
> According to GetVariable2() API, there is no guarantee that Data will be NULL,
> if error is returned.
> 
> I suggest we use status check instead of data pointer check.
> 

Yes, you are right. I will send out a V2 version of the series.

Best Regards,
Hao Wu

> 
> /**
>   Returns the status whether get the variable success. The function retrieves
>   variable  through the UEFI Runtime Service GetVariable().  The
>   returned buffer is allocated using AllocatePool().  The caller is responsible
>   for freeing this buffer with FreePool().
> 
>   If Name  is NULL, then ASSERT().
>   If Guid  is NULL, then ASSERT().
>   If Value is NULL, then ASSERT().
> 
>   @param[in]  Name  The pointer to a Null-terminated Unicode string.
>   @param[in]  Guid  The pointer to an EFI_GUID structure
>   @param[out] Value The buffer point saved the variable info.
>   @param[out] Size  The buffer size of the variable.
> 
>   @return EFI_OUT_OF_RESOURCES      Allocate buffer failed.
>   @return EFI_SUCCESS               Find the specified variable.
>   @return Others Errors             Return errors from call to gRT->GetVariable.
> 
> **/
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Friday, March 10, 2017 2:07 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> > GetVariable2 API
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > index fc0f8698a9..191a432369 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
> >                          NULL
> >                          );
> >        }
> > +      if (CapsuleResult != NULL) {
> > +        FreePool (CapsuleResult);
> > +      }
> >      }
> >
> >      // Lock it in normal boot path per UEFI spec.
> > --
> > 2.12.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-03-10  7:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-10  6:06 [PATCH 0/2] MdeModulePkg/CapsuleLib: Code refinements Hao Wu
2017-03-10  6:06 ` [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events Hao Wu
2017-03-10  6:13   ` Yao, Jiewen
2017-03-10  6:07 ` [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API Hao Wu
2017-03-10  6:16   ` Yao, Jiewen
2017-03-10  7:02     ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox