Re: [PATCH] uefi-sct/SctPkg:The Lun display order issue in iSCSI device path text

["Jin, Eric" <eric.jin@intel.com>]

Hi Supreeth,

The intention of the patch is to fix the LUN display issue in the iSCSI device path text.
In the UEFI spec, the according definition is clarified as " The LUN is an 8 byte array 
that is displayed in hexadecimal format with byte 0 first (i.e., on the left) and byte 7 
last (i.e, on the right), and is required. "

The current test has the mistake and need make sure the display order is correct.
The input is required to check the TEXT to binary system format, vice versa. 
It is the reason for the magic string "iSCSI(MyTargetName,0x12AB,0x0000005678000000,CRC32C,None,CHAP_BI,TCP)"
And the text format defined in the UEFI spec is:
iSCSI(TargetName, PortalGroup, LUN, HeaderDigest, DataDigest, Authentication, Protocol)

I will generate the new patch to do definition at first and then do initialization. Thank you for this comment.

Best Regards
Eric

-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com> 
Sent: Monday, October 15, 2018 10:23 AM
To: Jin, Eric <eric.jin@intel.com>; edk2-devel@lists.01.org
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: Re: [PATCH] uefi-sct/SctPkg:The Lun display order issue in iSCSI device path text



On 10/13/2018 05:33 PM, Eric Jin wrote:
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
>   .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c  | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git 
> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackB
> oxTest/DevicePathFromTextBBTestCoverage.c 
> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackB
> oxTest/DevicePathFromTextBBTestCoverage.c
> index fc099d8e..6f97924a 100644
> --- 
> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackB
> oxTest/DevicePathFromTextBBTestCoverage.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/Bl
> +++ ackBoxTest/DevicePathFromTextBBTestCoverage.c
> @@ -1,7 +1,7 @@
>   /** @file
>   
>     Copyright 2006 - 2017 Unified EFI, Inc.<BR>
> -  Copyright (c) 2010 - 2017, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2010 - 2018, 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 @@ -1442,7 +1442,7 @@ CreateiScsiDeviceNode (
>     CHAR16                      *DataDigestStr;
>     CHAR16                      *AuthenticationStr;
>     CHAR16                      *ProtocolStr;
> -  UINT64                      LunNum;
> +  UINT64                      LunNum = 0;
EFI coding convention does not allow initialization during definition.
>     ISCSI_DEVICE_PATH_WITH_NAME *iSCSI;
>   
>     NameStr           = SctSplitStr (&TextDeviceNode, L',');
> @@ -1459,7 +1459,7 @@ CreateiScsiDeviceNode (
>                                                           );
>     SctUnicodeToAscii (iSCSI->iSCSITargetName, NameStr, SctStrLen (NameStr));
>     iSCSI->TargetPortalGroupTag = (UINT16) SctStrToUInt 
> (PortalGroupStr);
> -  SctStrToUInt64 (LunStr, &LunNum);
> +  StrToUInt8Array(LunStr, &LunNum);
>     iSCSI->Lun = LunNum;
>   
>     Options = 0x0000;
> @@ -2846,12 +2846,12 @@ DevicePathFromTextConvertTextToDeviceNodeCoverageTest (
>                   (UINTN)__LINE__
>                   );
>     //
> -  // TDS 3.10.1.2.26
> +  // TDS 3.10.1.2.26   0x5678 - byte 3 is 0x56 and byte4 is 0x78
>     //
> -  SctStrCpy (text, 
> L"MyTargetName,0x12AB,5678,CRC32C,None,CHAP_BI,TCP");
> +  SctStrCpy (text, 
> + L"MyTargetName,0x12AB,0x0000005678000000,CRC32C,None,CHAP_BI,TCP");
Magic String.
>     pDevicePath = CreateiScsiDeviceNode(text);
>   
> -  SctStrCpy (text, 
> L"iSCSI(MyTargetName,0x12AB,5678,CRC32C,None,CHAP_BI,TCP)");
> +  SctStrCpy (text, 
> + L"iSCSI(MyTargetName,0x12AB,0x0000005678000000,CRC32C,None,CHAP_BI,T
> + CP)");
Magic String.
>     pReDevicePath = DevicePathFromText->ConvertTextToDeviceNode (text);
>     if (SctCompareMem (pDevicePath, pReDevicePath, SctDevicePathNodeLength ((EFI_DEVICE_PATH_PROTOCOL *) pReDevicePath)) == 0) {
>       AssertionType = EFI_TEST_ASSERTION_PASSED;