[edk2-devel] [PATCH v2 07/21] DynamicTablesPkg: Clear pointer in node creation fcts

PierreGondois posted 21 patches 4 years, 4 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 07/21] DynamicTablesPkg: Clear pointer in node creation fcts
Posted by PierreGondois 4 years, 4 months ago
From: Pierre Gondois <Pierre.Gondois@arm.com>

The following functions:
- AmlCreateRootNode()
- AmlCreateObjectNode()
- AmlCreateDataNode()
create a node and return it by populating a pointer. This pointer
should only be considered/used if the function returns successfully.
Otherwise, the value stored in this pointer should be ignored.

For their error handling, some other functions assume that this
pointer is reset to NULL if an error occurs during a node creation.
To make this assumption correct, exlpicitely clear this input pointer.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 10 ++++++++--
 .../AmlLib/CodeGen/AmlResourceDataCodeGen.c    | 10 ++++++++--
 .../Library/Common/AmlLib/Tree/AmlNode.c       | 18 +++++++++++++++---
 .../Library/Common/AmlLib/Tree/AmlNode.h       | 12 +++++++++---
 4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 5d310f201319..4775b68cd974 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -22,7 +22,9 @@
   @param [in]  Node           Newly created node.
   @param [in]  ParentNode     If provided, set ParentNode as the parent
                               of the node created.
-  @param [out] NewObjectNode  If success, contains the created object node.
+  @param [out] NewObjectNode  If not NULL:
+                               - and Success, contains the created Node.
+                               - and Error, reset to NULL.
 
   @retval  EFI_SUCCESS            The function completed successfully.
   @retval  EFI_INVALID_PARAMETER  Invalid parameter.
@@ -39,7 +41,7 @@ LinkNode (
   EFI_STATUS    Status;
 
   if (NewObjectNode != NULL) {
-    *NewObjectNode = Node;
+    *NewObjectNode = NULL;
   }
 
   // Add RdNode as the last element.
@@ -51,6 +53,10 @@ LinkNode (
     }
   }
 
+  if (NewObjectNode != NULL) {
+    *NewObjectNode = Node;
+  }
+
   return EFI_SUCCESS;
 }
 
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 089597a6c906..cba942c0fd1a 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -33,7 +33,9 @@
                             RdNode is then added at the end of the variable
                             list of resource data elements, but before the
                             "End Tag" Resource Data.
-  @param [out] NewRdNode    If not NULL, update the its value to RdNode.
+  @param [out] NewRdNode    If not NULL:
+                             - and Success, contains RdNode.
+                             - and Error, reset to NULL.
 
   @retval  EFI_SUCCESS            The function completed successfully.
   @retval  EFI_INVALID_PARAMETER  Invalid parameter.
@@ -52,7 +54,7 @@ LinkRdNode (
   AML_OBJECT_NODE   *BufferOpNode;
 
   if (NewRdNode != NULL) {
-    *NewRdNode = RdNode;
+    *NewRdNode = NULL;
   }
 
   if (ParentNode != NULL) {
@@ -85,6 +87,10 @@ LinkRdNode (
     }
   }
 
+  if (NewRdNode != NULL) {
+    *NewRdNode = RdNode;
+  }
+
   return Status;
 
 error_handler:
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
index 3740c0ac7bb8..300b07a2efe4 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
@@ -79,7 +79,9 @@ AmlDeleteRootNode (
 
   @param  [in]  SdtHeader       Pointer to an ACPI DSDT/SSDT header to copy
                                 the data from.
-  @param  [out] NewRootNodePtr  The created AML_ROOT_NODE.
+  @param  [out] NewRootNodePtr  If success, contains the created
+                                AML_ROOT_NODE.
+                                Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -101,6 +103,8 @@ AmlCreateRootNode (
     return EFI_INVALID_PARAMETER;
   }
 
+  *NewRootNodePtr = NULL;
+
   RootNode = AllocateZeroPool (sizeof (AML_ROOT_NODE));
   if (RootNode == NULL) {
     ASSERT (0);
@@ -163,7 +167,9 @@ AmlDeleteObjectNode (
   @param  [in]  PkgLength         PkgLength of the node if the AmlByteEncoding
                                   has the PkgLen attribute.
                                   0 otherwise.
-  @param  [out] NewObjectNodePtr  The created AML_OBJECT_NODE.
+  @param  [out] NewObjectNodePtr  If success, contains the created
+                                  AML_OBJECT_NODE.
+                                  Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -186,6 +192,8 @@ AmlCreateObjectNode (
     return EFI_INVALID_PARAMETER;
   }
 
+  *NewObjectNodePtr = NULL;
+
   ObjectNode = AllocateZeroPool (sizeof (AML_OBJECT_NODE));
   if (ObjectNode == NULL) {
     ASSERT (0);
@@ -252,7 +260,9 @@ AmlDeleteDataNode (
                                 this node. Data is copied from there.
   @param  [in]  DataSize        Number of bytes to consider at the address
                                 pointed by Data.
-  @param  [out] NewDataNodePtr  The created AML_DATA_NODE.
+  @param  [out] NewDataNodePtr  If success, contains the created
+                                AML_DATA_NODE.
+                                Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -284,6 +294,8 @@ AmlCreateDataNode (
     return EFI_INVALID_PARAMETER;
   }
 
+  *NewDataNodePtr = NULL;
+
   DataNode = AllocateZeroPool (sizeof (AML_DATA_NODE));
   if (DataNode == NULL) {
     ASSERT (0);
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
index 3584b572baae..465a0e2b4b2f 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
@@ -17,7 +17,9 @@
 
   @param  [in]  SdtHeader       Pointer to an ACPI DSDT/SSDT header to copy
                                 the data from.
-  @param  [out] NewRootNodePtr  The created AML_ROOT_NODE.
+  @param  [out] NewRootNodePtr  If success, contains the created
+                                AML_ROOT_NODE.
+                                Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -36,7 +38,9 @@ AmlCreateRootNode (
   @param  [in]  PkgLength         PkgLength of the node if the AmlByteEncoding
                                   has the PkgLen attribute.
                                   0 otherwise.
-  @param  [out] NewObjectNodePtr  The created AML_OBJECT_NODE.
+  @param  [out] NewObjectNodePtr  If success, contains the created
+                                  AML_OBJECT_NODE.
+                                  Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -57,7 +61,9 @@ AmlCreateObjectNode (
                                 this node. Data is copied from there.
   @param  [in]  DataSize        Number of bytes to consider at the address
                                 pointed by Data.
-  @param  [out] NewDataNodePtr  The created AML_DATA_NODE.
+  @param  [out] NewDataNodePtr  If success, contains the created
+                                AML_DATA_NODE.
+                                Otherwise reset to NULL.
 
   @retval EFI_SUCCESS             The function completed successfully.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81586): https://edk2.groups.io/g/devel/message/81586
Mute This Topic: https://groups.io/mt/86148196/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 07/21] DynamicTablesPkg: Clear pointer in node creation fcts
Posted by Sami Mujawar 4 years, 4 months ago
Hi Pierre,

There is a minor typo in the commit message that I will fix when pushing
the patch.

Otherwise, this patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 07/10/2021 04:31 PM, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> The following functions:
> - AmlCreateRootNode()
> - AmlCreateObjectNode()
> - AmlCreateDataNode()
> create a node and return it by populating a pointer. This pointer
> should only be considered/used if the function returns successfully.
> Otherwise, the value stored in this pointer should be ignored.
>
> For their error handling, some other functions assume that this
> pointer is reset to NULL if an error occurs during a node creation.
> To make this assumption correct, exlpicitely clear this input pointer.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>   .../Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 10 ++++++++--
>   .../AmlLib/CodeGen/AmlResourceDataCodeGen.c    | 10 ++++++++--
>   .../Library/Common/AmlLib/Tree/AmlNode.c       | 18 +++++++++++++++---
>   .../Library/Common/AmlLib/Tree/AmlNode.h       | 12 +++++++++---
>   4 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index 5d310f201319..4775b68cd974 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -22,7 +22,9 @@
>     @param [in]  Node           Newly created node.
>     @param [in]  ParentNode     If provided, set ParentNode as the parent
>                                 of the node created.
> -  @param [out] NewObjectNode  If success, contains the created object node.
> +  @param [out] NewObjectNode  If not NULL:
> +                               - and Success, contains the created Node.
> +                               - and Error, reset to NULL.
>
>     @retval  EFI_SUCCESS            The function completed successfully.
>     @retval  EFI_INVALID_PARAMETER  Invalid parameter.
> @@ -39,7 +41,7 @@ LinkNode (
>     EFI_STATUS    Status;
>
>     if (NewObjectNode != NULL) {
> -    *NewObjectNode = Node;
> +    *NewObjectNode = NULL;
>     }
>
>     // Add RdNode as the last element.
> @@ -51,6 +53,10 @@ LinkNode (
>       }
>     }
>
> +  if (NewObjectNode != NULL) {
> +    *NewObjectNode = Node;
> +  }
> +
>     return EFI_SUCCESS;
>   }
>
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
> index 089597a6c906..cba942c0fd1a 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
> @@ -33,7 +33,9 @@
>                               RdNode is then added at the end of the variable
>                               list of resource data elements, but before the
>                               "End Tag" Resource Data.
> -  @param [out] NewRdNode    If not NULL, update the its value to RdNode.
> +  @param [out] NewRdNode    If not NULL:
> +                             - and Success, contains RdNode.
> +                             - and Error, reset to NULL.
>
>     @retval  EFI_SUCCESS            The function completed successfully.
>     @retval  EFI_INVALID_PARAMETER  Invalid parameter.
> @@ -52,7 +54,7 @@ LinkRdNode (
>     AML_OBJECT_NODE   *BufferOpNode;
>
>     if (NewRdNode != NULL) {
> -    *NewRdNode = RdNode;
> +    *NewRdNode = NULL;
>     }
>
>     if (ParentNode != NULL) {
> @@ -85,6 +87,10 @@ LinkRdNode (
>       }
>     }
>
> +  if (NewRdNode != NULL) {
> +    *NewRdNode = RdNode;
> +  }
> +
>     return Status;
>
>   error_handler:
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
> index 3740c0ac7bb8..300b07a2efe4 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.c
> @@ -79,7 +79,9 @@ AmlDeleteRootNode (
>
>     @param  [in]  SdtHeader       Pointer to an ACPI DSDT/SSDT header to copy
>                                   the data from.
> -  @param  [out] NewRootNodePtr  The created AML_ROOT_NODE.
> +  @param  [out] NewRootNodePtr  If success, contains the created
> +                                AML_ROOT_NODE.
> +                                Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -101,6 +103,8 @@ AmlCreateRootNode (
>       return EFI_INVALID_PARAMETER;
>     }
>
> +  *NewRootNodePtr = NULL;
> +
>     RootNode = AllocateZeroPool (sizeof (AML_ROOT_NODE));
>     if (RootNode == NULL) {
>       ASSERT (0);
> @@ -163,7 +167,9 @@ AmlDeleteObjectNode (
>     @param  [in]  PkgLength         PkgLength of the node if the AmlByteEncoding
>                                     has the PkgLen attribute.
>                                     0 otherwise.
> -  @param  [out] NewObjectNodePtr  The created AML_OBJECT_NODE.
> +  @param  [out] NewObjectNodePtr  If success, contains the created
> +                                  AML_OBJECT_NODE.
> +                                  Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -186,6 +192,8 @@ AmlCreateObjectNode (
>       return EFI_INVALID_PARAMETER;
>     }
>
> +  *NewObjectNodePtr = NULL;
> +
>     ObjectNode = AllocateZeroPool (sizeof (AML_OBJECT_NODE));
>     if (ObjectNode == NULL) {
>       ASSERT (0);
> @@ -252,7 +260,9 @@ AmlDeleteDataNode (
>                                   this node. Data is copied from there.
>     @param  [in]  DataSize        Number of bytes to consider at the address
>                                   pointed by Data.
> -  @param  [out] NewDataNodePtr  The created AML_DATA_NODE.
> +  @param  [out] NewDataNodePtr  If success, contains the created
> +                                AML_DATA_NODE.
> +                                Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -284,6 +294,8 @@ AmlCreateDataNode (
>       return EFI_INVALID_PARAMETER;
>     }
>
> +  *NewDataNodePtr = NULL;
> +
>     DataNode = AllocateZeroPool (sizeof (AML_DATA_NODE));
>     if (DataNode == NULL) {
>       ASSERT (0);
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
> index 3584b572baae..465a0e2b4b2f 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNode.h
> @@ -17,7 +17,9 @@
>
>     @param  [in]  SdtHeader       Pointer to an ACPI DSDT/SSDT header to copy
>                                   the data from.
> -  @param  [out] NewRootNodePtr  The created AML_ROOT_NODE.
> +  @param  [out] NewRootNodePtr  If success, contains the created
> +                                AML_ROOT_NODE.
> +                                Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -36,7 +38,9 @@ AmlCreateRootNode (
>     @param  [in]  PkgLength         PkgLength of the node if the AmlByteEncoding
>                                     has the PkgLen attribute.
>                                     0 otherwise.
> -  @param  [out] NewObjectNodePtr  The created AML_OBJECT_NODE.
> +  @param  [out] NewObjectNodePtr  If success, contains the created
> +                                  AML_OBJECT_NODE.
> +                                  Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -57,7 +61,9 @@ AmlCreateObjectNode (
>                                   this node. Data is copied from there.
>     @param  [in]  DataSize        Number of bytes to consider at the address
>                                   pointed by Data.
> -  @param  [out] NewDataNodePtr  The created AML_DATA_NODE.
> +  @param  [out] NewDataNodePtr  If success, contains the created
> +                                AML_DATA_NODE.
> +                                Otherwise reset to NULL.
>
>     @retval EFI_SUCCESS             The function completed successfully.
>     @retval EFI_INVALID_PARAMETER   Invalid parameter.

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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81679): https://edk2.groups.io/g/devel/message/81679
Mute This Topic: https://groups.io/mt/86148196/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-