Re: [PATCH v3][Patch 2/4] MdePkg/UefiDevicePathLib: Add DevPathFromTextDns and DevPathToTextDns libraries

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Thanks the comments, I agree it's better to check the returned value of CreateDeviceNode.

For the Dns, I can refine the code before the commit. As for the others, we'd better leave it to the owner for the decision whether to fix it or not.

Thanks,
Jiaxin

> -----Original Message-----
> From: Ye, Ting
> Sent: Monday, August 14, 2017 11:28 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [PATCH v3][Patch 2/4] MdePkg/UefiDevicePathLib: Add
> DevPathFromTextDns and DevPathToTextDns libraries
> 
> Hi Jiaxin,
> 
> Shall we check the return value of CreateDeviceNode in
> DevPathFromTextDns to make sure we don't use NULL pointer?
> I see many internal functions DevPathFromText** in DevicePathFromText.c
> NOT check this. Any particular reason we don't need this?
> 
> Thanks,
> Ting
> 
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Thursday, August 03, 2017 8:20 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v3][Patch 2/4] MdePkg/UefiDevicePathLib: Add
> DevPathFromTextDns and DevPathToTextDns libraries
> 
> V3:
> * Fix the bug in DevPathFromTextDns()
> 
> V2:
> * Add no IP instance case check.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  .../Library/UefiDevicePathLib/DevicePathFromText.c | 90
> ++++++++++++++++++++++
>  .../Library/UefiDevicePathLib/DevicePathToText.c   | 46 +++++++++++
>  2 files changed, 136 insertions(+)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> index f50c11c..0fb703b 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> @@ -2723,10 +2723,99 @@ DevPathFromTextBluetoothLE (
>      );
>    return (EFI_DEVICE_PATH_PROTOCOL *) BluetoothLeDp;  }
> 
>  /**
> +  Converts a text device path node to DNS device path structure.
> +
> +  @param TextDeviceNode  The input Text device path node.
> +
> +  @return A pointer to the newly-created DNS device path structure.
> +
> +**/
> +EFI_DEVICE_PATH_PROTOCOL *
> +DevPathFromTextDns (
> +  IN CHAR16 *TextDeviceNode
> +  )
> +{
> +  CHAR16            *DeviceNodeStr;
> +  CHAR16            *DeviceNodeStrPtr;
> +  UINT32            DnsServerIpCount;
> +  UINT16            DnsDeviceNodeLength;
> +  DNS_DEVICE_PATH   *DnsDeviceNode;
> +  UINT32            DnsServerIpIndex;
> +  CHAR16            *DnsServerIp;
> +
> +
> +  //
> +  // Count the DNS server address number.
> +  //
> +  DeviceNodeStr = UefiDevicePathLibStrDuplicate (TextDeviceNode);  if
> + (DeviceNodeStr == NULL) {
> +    return NULL;
> +  }
> +
> +  DeviceNodeStrPtr = DeviceNodeStr;
> +
> +  DnsServerIpCount = 0;
> +  while (DeviceNodeStrPtr != NULL && *DeviceNodeStrPtr != L'\0') {
> +    GetNextParamStr (&DeviceNodeStrPtr);
> +    DnsServerIpCount ++;
> +  }
> +
> +  FreePool (DeviceNodeStr);
> +  DeviceNodeStr = NULL;
> +
> +  //
> +  // One or more instances of the DNS server address in EFI_IP_ADDRESS,
> + // otherwise, NULL will be returned.
> +  //
> +  if (DnsServerIpCount == 0) {
> +    return NULL;
> +  }
> +
> +  //
> +  // Create the DNS DeviceNode.
> +  //
> +  DnsDeviceNodeLength = (UINT16) (sizeof (EFI_DEVICE_PATH_PROTOCOL)
> + sizeof (UINT8) + DnsServerIpCount * sizeof (EFI_IP_ADDRESS));
> +  DnsDeviceNode       = (DNS_DEVICE_PATH *) CreateDeviceNode (
> +                                              MESSAGING_DEVICE_PATH,
> +                                              MSG_DNS_DP,
> +                                              DnsDeviceNodeLength
> +                                              );
> +
> +  //
> +  // Confirm the DNS server address is IPv4 or IPv6 type.
> +  //
> +  DeviceNodeStrPtr = TextDeviceNode;
> +  while (!IS_NULL (*DeviceNodeStrPtr)) {
> +    if (*DeviceNodeStrPtr == L'.') {
> +      DnsDeviceNode->IsIPv6 = 0x00;
> +      break;
> +    }
> +
> +    if (*DeviceNodeStrPtr == L':') {
> +      DnsDeviceNode->IsIPv6 = 0x01;
> +      break;
> +    }
> +
> +    DeviceNodeStrPtr++;
> +  }
> +
> +  for (DnsServerIpIndex = 0; DnsServerIpIndex < DnsServerIpCount;
> DnsServerIpIndex++) {
> +    DnsServerIp = GetNextParamStr (&TextDeviceNode);
> +    if (DnsDeviceNode->IsIPv6 == 0x00) {
> +      StrToIpv4Address (DnsServerIp,  NULL, &(DnsDeviceNode-
> >DnsServerIp[DnsServerIpIndex].v4), NULL);
> +    } else {
> +      StrToIpv6Address (DnsServerIp, NULL, &(DnsDeviceNode-
> >DnsServerIp[DnsServerIpIndex].v6), NULL);
> +    }
> +  }
> +
> +  return (EFI_DEVICE_PATH_PROTOCOL *) DnsDeviceNode; }
> +
> +/**
>    Converts a text device path node to URI device path structure.
> 
>    @param TextDeviceNode  The input Text device path node.
> 
>    @return A pointer to the newly-created URI device path structure.
> @@ -3395,10 +3484,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> DEVICE_PATH_FROM_TEXT_TABLE mUefiDevicePathLibDevP
>    {L"UsbTestAndMeasurement",
> DevPathFromTextUsbTestAndMeasurement   },
>    {L"UsbWwid",                 DevPathFromTextUsbWwid                 },
>    {L"Unit",                    DevPathFromTextUnit                    },
>    {L"iSCSI",                   DevPathFromTextiSCSI                   },
>    {L"Vlan",                    DevPathFromTextVlan                    },
> +  {L"Dns",                     DevPathFromTextDns                     },
>    {L"Uri",                     DevPathFromTextUri                     },
>    {L"Bluetooth",               DevPathFromTextBluetooth               },
>    {L"Wi-Fi",                   DevPathFromTextWiFi                    },
>    {L"BluetoothLE",             DevPathFromTextBluetoothLE             },
>    {L"MediaPath",               DevPathFromTextMediaPath               },
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> index b8d9491..63542db 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> @@ -1695,10 +1695,55 @@ DevPathToTextBluetoothLE (
>      BluetoothLE->Address.Type
>      );
>  }
> 
>  /**
> +  Converts a DNS device path structure to its string representative.
> +
> +  @param Str             The string representative of input device.
> +  @param DevPath         The input device path structure.
> +  @param DisplayOnly     If DisplayOnly is TRUE, then the shorter text
> representation
> +                         of the display node is used, where applicable. If DisplayOnly
> +                         is FALSE, then the longer text representation of the display
> node
> +                         is used.
> +  @param AllowShortcuts  If AllowShortcuts is TRUE, then the shortcut
> forms of text
> +                         representation for a device node can be used, where
> applicable.
> +
> +**/
> +VOID
> +DevPathToTextDns (
> +  IN OUT POOL_PRINT  *Str,
> +  IN VOID            *DevPath,
> +  IN BOOLEAN         DisplayOnly,
> +  IN BOOLEAN         AllowShortcuts
> +  )
> +{
> +  DNS_DEVICE_PATH  *DnsDevPath;
> +  UINT32           DnsServerIpCount;
> +  UINT32           DnsServerIpIndex;
> +
> +  DnsDevPath     = DevPath;
> +  DnsServerIpCount = (UINT32) (DevicePathNodeLength(DnsDevPath) -
> + sizeof (EFI_DEVICE_PATH_PROTOCOL) - sizeof (DnsDevPath->IsIPv6)) /
> + sizeof (EFI_IP_ADDRESS);
> +
> +  UefiDevicePathLibCatPrint (Str, L"Dns(");
> +
> +  for (DnsServerIpIndex = 0; DnsServerIpIndex < DnsServerIpCount;
> DnsServerIpIndex++) {
> +    if (DnsDevPath->IsIPv6 == 0x00) {
> +      CatIPv4Address (Str, &(DnsDevPath-
> >DnsServerIp[DnsServerIpIndex].v4));
> +    } else {
> +      CatIPv6Address (Str, &(DnsDevPath-
> >DnsServerIp[DnsServerIpIndex].v6));
> +    }
> +
> +    if (DnsServerIpIndex < DnsServerIpCount - 1) {
> +      UefiDevicePathLibCatPrint (Str, L",");
> +    }
> +  }
> +
> +  UefiDevicePathLibCatPrint (Str, L")"); }
> +
> +/**
>    Converts a URI device path structure to its string representative.
> 
>    @param Str             The string representative of input device.
>    @param DevPath         The input device path structure.
>    @param DisplayOnly     If DisplayOnly is TRUE, then the shorter text
> representation
> @@ -2223,10 +2268,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED const
> DEVICE_PATH_TO_TEXT_TABLE mUefiDevicePathLib
>    {MESSAGING_DEVICE_PATH, MSG_INFINIBAND_DP,
> DevPathToTextInfiniBand     },
>    {MESSAGING_DEVICE_PATH, MSG_UART_DP,
> DevPathToTextUart           },
>    {MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
> DevPathToTextVendor         },
>    {MESSAGING_DEVICE_PATH, MSG_ISCSI_DP,
> DevPathToTextiSCSI          },
>    {MESSAGING_DEVICE_PATH, MSG_VLAN_DP,
> DevPathToTextVlan           },
> +  {MESSAGING_DEVICE_PATH, MSG_DNS_DP,
> DevPathToTextDns            },
>    {MESSAGING_DEVICE_PATH, MSG_URI_DP,
> DevPathToTextUri            },
>    {MESSAGING_DEVICE_PATH, MSG_BLUETOOTH_DP,
> DevPathToTextBluetooth      },
>    {MESSAGING_DEVICE_PATH, MSG_WIFI_DP,
> DevPathToTextWiFi           },
>    {MESSAGING_DEVICE_PATH, MSG_BLUETOOTH_LE_DP,
> DevPathToTextBluetoothLE    },
>    {MEDIA_DEVICE_PATH,     MEDIA_HARDDRIVE_DP,
> DevPathToTextHardDrive      },
> --
> 1.9.5.msysgit.1