Hi Heinrich,

I guess it is not allowed to make them the same now, so I submitted V2, added your recomended fix message to the commit message, and also added more reviewers.

Thanks,
Chao

On 2024/2/27 09:15, Chao Li wrote:

Hi Heinrich,

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.

_._,_._,_