From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 86FD9AC0B57 for ; Tue, 8 Aug 2023 04:31:01 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=FvIaDfm9DFR0bEtf1DWwDJA7YQp9OPJEHeHI843yoDU=; c=relaxed/simple; d=groups.io; h=Subject:To:From:User-Agent:MIME-Version:Date:References:In-Reply-To:Message-ID:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1691469060; v=1; b=WfFHblXcIbu8g0wm5MjO/ipO9UM1d0NguODS8i4btQjHqzo+g8mmoTmw0LxZmhgHWorNMamt uKbQM0uL4EjSYGRDPOw3EIg5h+oustWW25zj4URdvkHq5CXXhjMuAQWwRF0+/gGq3zZTPMxd0bv OD+yk2hp9mm/xBQSgelGg+b0= X-Received: by 127.0.0.2 with SMTP id 6QNHYY7687511x16PdLHlFCX; Mon, 07 Aug 2023 21:31:00 -0700 Subject: Re: [edk2-devel] [PATCH v2 2/4] DynamicTablesPkg: Add support for simple method invocation. To: Jeff Brasen ,devel@edk2.groups.io From: "Sami Mujawar" X-Originating-Location: Bengaluru, Karnataka, IN (223.233.85.32) X-Originating-Platform: Mac Firefox 115 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Mon, 07 Aug 2023 21:30:59 -0700 References: <124c8bb20d692f1b7c18758e96dbf35ed17478f6.1689027745.git.jbrasen@nvidia.com> In-Reply-To: <124c8bb20d692f1b7c18758e96dbf35ed17478f6.1689027745.git.jbrasen@nvidia.com> Message-ID: <16750.1691469059390738761@groups.io> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,sami.mujawar@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: Nc58cLYCVliIZmiNqqYQELMpx7686176AA= Content-Type: multipart/alternative; boundary="xGASE59QuffPNmbh24mT" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=WfFHblXc; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --xGASE59QuffPNmbh24mT Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Jeff, Apologies for the delay in feedback. Please see my response inline. On Mon, Jul 10, 2023 at 03:26 PM, Jeff Brasen wrote: >=20 > + > + // AmlCodeGenReturn() deletes DataNode if error. > + Status =3D AmlCodeGenReturn ( > + (AML_NODE_HEADER *)DataNode, > + ParentNode, > + &ObjectNode > + ); > + ASSERT_EFI_ERROR (Status); I think the Status should be checked and error needs to be handled to free = AmlNameString and IntNode. Can you check, please? >=20 > + > + Status =3D AmlVarListAddTail ( > + (AML_NODE_HANDLE)ObjectNode, > + (AML_NODE_HANDLE)IntNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto exit_handler; I think IntNode would need to be freed as well. Can you check, please? >=20 > + } > + Your sign-off is also missing. Otherwise this patch looks good to me. With the error handling addressed, Reviewed-by: Sami Mujawar Regards, Sami Mujawar -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107629): https://edk2.groups.io/g/devel/message/107629 Mute This Topic: https://groups.io/mt/100068077/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --xGASE59QuffPNmbh24mT Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Jeff,

Apologies for the delay in feedback.
Please see m= y response inline.

On Mon, Jul 10, 2023 at 03:26 PM, Jeff Brasen= wrote:
+
+ // AmlCodeGenReturn() deletes DataNode if error.
= + Status =3D AmlCodeGenReturn (
+ (AML_NODE_HEADER *)DataNode,
+ = ParentNode,
+ &ObjectNode
+ );
+ ASSERT_EFI_ERROR (Statu= s);
I think the Status should be checked and error needs to be handled to free = AmlNameString and IntNode. Can you check, please?
+
+ Status =3D AmlVarListAddTail (
+ (AML_NODE_HANDLE= )ObjectNode,
+ (AML_NODE_HANDLE)IntNode
+ );
+ if (EFI_ERROR= (Status)) {
+ ASSERT (0);
+ goto exit_handler;
I think IntNode would need to be freed as well. Can you check, please?
+ }
+

Your sign-off is also missing. Otherwise this patch looks good to me.=

With the error handling addressed,
Reviewed-by: Sami Muja= war <sami.mujawar@arm.com>

Regards,

Sami Mujawa= r
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#107629) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--xGASE59QuffPNmbh24mT--