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 7D120D80D0C for ; Tue, 27 Feb 2024 01:15:52 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=7/m8/Jo/+DEI+XIsezCJPAlhSzQsNdy+HJkW/mlrwq8=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1708996551; v=1; b=qY3Ayvqo2ITfzMDHI5ZDYAN3B2xNAi/liu4M+vgHZdQhql4rzwk3amcbHPfn7aZqiqyXq5/k G7F+stGv82BRtr5hgVnsuhoFOLVFcUTFTe94bDaIvbFJKLmkoATmGo/q/47DlpJkubl6/gyv+Mg eJg8da9wadUovphKF+P2LVME= X-Received: by 127.0.0.2 with SMTP id oJYrYY7687511xjrbzPlzyQx; Mon, 26 Feb 2024 17:15:51 -0800 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web11.2460.1708996549373515100 for ; Mon, 26 Feb 2024 17:15:50 -0800 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8DxK+nBN91luMwRAA--.26782S3; Tue, 27 Feb 2024 09:15:45 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxZMy7N91l+9ZGAA--.62307S3; Tue, 27 Feb 2024 09:15:39 +0800 (CST) Message-ID: Date: Tue, 27 Feb 2024 09:15:39 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c To: devel@edk2.groups.io, heinrich.schuchardt@canonical.com Cc: G Edhaya Chandran , Barton Gao , Carolyn Gjertsen References: <20240226101752.247067-1-lichao@...> From: "Chao Li" In-Reply-To: X-CM-TRANSID: AQAAf8CxZMy7N91l+9ZGAA--.62307S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQAACGXcS+QFKQADsw X-Coremail-Antispam: 1Uk129KBj93XoWxZFWUuF4DtFWktF18Jr45XFc_yoWrJFyUpF n5XFyjvFZFqw1I9an0ka1xKrykXr4kZ3W8Ga15Wayjkrn7JFZIvFW8ur9Ygw1UGr4kZr4Y qryjq39rGF90kwcCm3ZEXasCq-sJn29KB7ZKAUJUUUU7529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3UbIjqfuFe4nvWSU5nxnvy29KBjDU0xBIdaVrnRJUUUvvb4IE77IF4wAF F20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r 1Y6r17M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAF wI0_Gr0_Xr1l84ACjcxK6xIIjxv20xvEc7CjxVAFwI0_Gr0_Cr1l84ACjcxK6I8E87Iv67 AKxVW8JVWxJwA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1UM2kKe7AKxVWUXVWUAwAS 0I0E0xvYzxvE52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMcIj6xIIjxv20x vE14v26r126r1DMcIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr0_Gr1lF7xv r2IY64vIr41l7480Y4vEI4kI2Ix0rVAqx4xJMxAIw28IcxkI7VAKI48JMxC20s026xCaFV Cjc4AY6r1j6r4UMxCIbckI1I0E14v26r1Y6r17MI8I3I0E5I8CrVAFwI0_JrI_JrWlx2Iq xVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUAVWUtwCIc40Y0x0EwIxGrwCI42 IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x0267AKxVWUJVW8JwCI42IY 6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr0_Gr1lIxAIcVC2z280aV CY1x0267AKxVWUJVW8JbIYCTnIWIevJa73UjIFyTuYvjxU7vtCUUUUU 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,lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: ufWo6Cr9AIj3OxTHqtw1ct93x7686176AA= Content-Type: multipart/alternative; boundary="------------un9LyWd0GtWRFYZFBmloxSj5" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=qY3Ayvqo; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=none --------------un9LyWd0GtWRFYZFBmloxSj5 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Heinrich, Thanks, Chao On 2024/2/26 19:02, Heinrich Schuchardt wrote: > On 26.02.24 11:17, Chao Li wrote: >> DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a >> bug, code: >> >> SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> >> DNS is a pointer, which is increased by a structure size and converted >> to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it. > > Hello Chao, > > thanks for diving into this. > > Please, add > > Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct > -Wincompatible-pointer-types warnings") > >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712 >> >> Cc: G Edhaya Chandran >> Cc: Barton Gao >> Cc: Carolyn Gjertsen >> Signed-off-by: Chao Li >> --- >>   .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c       | 8 ++++---- >>   1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> >> index c96ee246..bd11c25a 100644 >> --- >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> +++ >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( >>     } >> >>     if (DNS->IsIPv6 == 0) { >> -    SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> -    SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> +    SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH))); >> +    SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >>     } >> >>     if (DNS->IsIPv6 == 1) { >> -    SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> -    SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> +    SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH))); >> +    SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); > > The change looks correct. > > I would prefer using the fields of the DNS_DEVICE_PATH typedef from > MdePkg/Include/Protocol/DevicePath.h to avoid the conversions: > > > +    SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4) > > +    SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4) > > > +    SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6) > > +    SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6) I also prefer this way, but the definition of DNS in UEFI/Protocol/DevicePath.h is not same as the MdePkg version, should we make them the same? > > Best regards > > Heinrich > >>     } >> >>     return (EFI_DEVICE_PATH_PROTOCOL *) DNS; >> -- >> 2.27.0 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115995): https://edk2.groups.io/g/devel/message/115995 Mute This Topic: https://groups.io/mt/104579419/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- --------------un9LyWd0GtWRFYZFBmloxSj5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Heinrich,


Thanks,
Chao
On 2024/2/26 19:02, Heinrich Schuchardt wrote:
On 26.02.24 11:17, Chao Li wrote:
DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a
bug, code:

SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH)));

DNS is a pointer, which is increased by a structure size and converted
to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it.

Hello Chao,

thanks for diving into this.

Please, add

Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct -Wincompatible-pointer-types warnings")


BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712

Cc: G Edhaya Chandran <Edhaya.Chandran@...>
Cc: Barton Gao <gaojie@...>
Cc: Carolyn Gjertsen <Carolyn.Gjertsen@...>
Signed-off-by: Chao Li <lichao@...>
---
  .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c       | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
index c96ee246..bd11c25a 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
@@ -1734,13 +1734,13 @@ CreateDNSDeviceNode (
    }

    if (DNS->IsIPv6 == 0) {
-    SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH)));
-    SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
+    SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH)));
+    SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
    }

    if (DNS->IsIPv6 == 1) {
-    SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH)));
-    SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
+    SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH)));
+    SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));

The change looks correct.

I would prefer using the fields of the DNS_DEVICE_PATH typedef from MdePkg/Include/Protocol/DevicePath.h to avoid the conversions:

> +    SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4)
> +    SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4)

> +    SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6)
> +    SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6)
I also prefer this way, but the definition of DNS in UEFI/Protocol/DevicePath.h is not same as the MdePkg version, should we make them the same?

Best regards

Heinrich

    }

    return (EFI_DEVICE_PATH_PROTOCOL *) DNS;
--
2.27.0





_._,_._,_
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#115995) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------un9LyWd0GtWRFYZFBmloxSj5--