From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Wang, Fan" <fan.wang@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Fu, Siyuan" <siyuan.fu@intel.com>, "Ye, Ting" <ting.ye@intel.com>
Subject: Re: [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.
Date: Wed, 3 Jan 2018 06:03:19 +0000 [thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B727416358D9F@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1514946711-4056-3-git-send-email-fan.wang@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: Wang, Fan
> Sent: Wednesday, January 3, 2018 10:32 AM
> To: edk2-devel@lists.01.org
> Cc: Wang, Fan <fan.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye,
> Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and
> ASSERT handling.
>
> From: Wang Fan <fan.wang@intel.com>
>
> * Library API should check the input parameters before use, or
> ASSERT to tell it has to meet some requirements. But in DxeNetLib,
> not all functions follows this rule.
> * ASSERT shouldn't be used as error handling, add some handling code
> for errors.
> * Add some ASSERT commence in function notes.
>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 119
> +++++++++++++++++++++++++----
> 1 file changed, 105 insertions(+), 14 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 0f00f79..90f17b7 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -1,9 +1,9 @@
> /** @file
> Network library.
>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> http://opensource.org/licenses/bsd-license.php
> @@ -196,10 +196,11 @@ SyslogLocateSnp (
> Transmit a syslog packet synchronously through SNP. The Packet
> already has the ethernet header prepended. This function should
> fill in the source MAC because it will try to locate a SNP each
> time it is called to avoid the problem if SNP is unloaded.
> This code snip is copied from MNP.
> + If Packet is NULL, then ASSERT().
>
> @param[in] Packet The Syslog packet
> @param[in] Length The length of the packet
>
> @retval EFI_DEVICE_ERROR Failed to locate a usable SNP protocol
> @@ -217,10 +218,12 @@ SyslogSendPacket (
> ETHER_HEAD *Ether;
> EFI_STATUS Status;
> EFI_EVENT TimeoutEvent;
> UINT8 *TxBuf;
>
> + ASSERT (Packet != NULL);
> +
> Snp = SyslogLocateSnp ();
>
> if (Snp == NULL) {
> return EFI_DEVICE_ERROR;
> }
> @@ -308,11 +311,11 @@ ON_EXIT:
> @param[in] Line The line of code in the File that contains the current log
> @param[in] Message The log message
> @param[in] BufLen The lenght of the Buf
> @param[out] Buf The buffer to put the packet data
>
> - @return The length of the syslog packet built.
> + @return The length of the syslog packet built, 0 represents no packet is
> built.
>
> **/
> UINT32
> SyslogBuildPacket (
> IN UINT32 Level,
> @@ -322,10 +325,11 @@ SyslogBuildPacket (
> IN UINT8 *Message,
> IN UINT32 BufLen,
> OUT CHAR8 *Buf
> )
> {
> + EFI_STATUS Status;
> ETHER_HEAD *Ether;
> IP4_HEAD *Ip4;
> EFI_UDP_HEADER *Udp4;
> EFI_TIME Time;
> UINT32 Pri;
> @@ -377,12 +381,14 @@ SyslogBuildPacket (
>
> //
> // Build the syslog message body with <PRI> Timestamp machine module
> Message
> //
> Pri = ((NET_SYSLOG_FACILITY & 31) << 3) | (Level & 7);
> - gRT->GetTime (&Time, NULL);
> - ASSERT ((Time.Month <= 12) && (Time.Month >= 1));
> + Status = gRT->GetTime (&Time, NULL);
> + if (EFI_ERROR (Status)) {
> + return 0;
> + }
>
> //
> // Use %a to format the ASCII strings, %s to format UNICODE strings
> //
> Len = 0;
> @@ -437,10 +443,12 @@ SyslogBuildPacket (
> __FILE__,
> __LINE__,
> NetDebugASPrint ("State transit to %a\n", Name)
> )
>
> + If Format is NULL, then ASSERT().
> +
> @param Format The ASCII format string.
> @param ... The variable length parameter whose format is determined
> by the Format string.
>
> @return The buffer containing the formatted message,
> @@ -455,10 +463,12 @@ NetDebugASPrint (
> )
> {
> VA_LIST Marker;
> CHAR8 *Buf;
>
> + ASSERT (Format != NULL);
> +
> Buf = (CHAR8 *) AllocatePool (NET_DEBUG_MSG_LEN);
>
> if (Buf == NULL) {
> return NULL;
> }
> @@ -481,11 +491,12 @@ NetDebugASPrint (
> @param File The file that contains the log.
> @param Line The exact line that contains the log.
> @param Message The user message to log.
>
> @retval EFI_INVALID_PARAMETER Any input parameter is invalid.
> - @retval EFI_OUT_OF_RESOURCES Failed to allocate memory for the
> packet
> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory for the
> packet.
> + @retval EFI_DEVICE_ERROR Device error occurs.
> @retval EFI_SUCCESS The log is discard because that it is more verbose
> than the mNetDebugLevelMax. Or, it has been sent out.
> **/
> EFI_STATUS
> EFIAPI
> @@ -502,11 +513,11 @@ NetDebugOutput (
> EFI_STATUS Status;
>
> //
> // Check whether the message should be sent out
> //
> - if (Message == NULL) {
> + if (Message == NULL || File == NULL || Module == NULL) {
> return EFI_INVALID_PARAMETER;
> }
>
> if (Level > mNetDebugLevelMax) {
> Status = EFI_SUCCESS;
> @@ -535,13 +546,17 @@ NetDebugOutput (
> Line,
> Message,
> NET_SYSLOG_PACKET_LEN,
> Packet
> );
> + if (Len == 0) {
> + Status = EFI_DEVICE_ERROR;
> + } else {
> + mSyslogPacketSeq++;
> + Status = SyslogSendPacket (Packet, Len);
> + }
>
> - mSyslogPacketSeq++;
> - Status = SyslogSendPacket (Packet, Len);
> FreePool (Packet);
>
> ON_EXIT:
> FreePool (Message);
> return Status;
> @@ -673,10 +688,12 @@ NetIp4IsUnicast (
> }
>
> /**
> Check whether the incoming IPv6 address is a valid unicast address.
>
> + ASSERT if Ip6 is NULL.
> +
> If the address is a multicast address has binary 0xFF at the start, it is not
> a valid unicast address. If the address is unspecified ::, it is not a valid
> unicast address to be assigned to any node. If the address is loopback
> address
> ::1, it is also not a valid unicast address to be assigned to any physical
> interface.
> @@ -693,10 +710,12 @@ NetIp6IsValidUnicast (
> )
> {
> UINT8 Byte;
> UINT8 Index;
>
> + ASSERT (Ip6 != NULL);
> +
> if (Ip6->Addr[0] == 0xFF) {
> return FALSE;
> }
>
> for (Index = 0; Index < 15; Index++) {
> @@ -715,10 +734,12 @@ NetIp6IsValidUnicast (
> }
>
> /**
> Check whether the incoming Ipv6 address is the unspecified address or not.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - Yes, unspecified
> @retval FALSE - No
>
> @@ -729,10 +750,12 @@ NetIp6IsUnspecifiedAddr (
> IN EFI_IPv6_ADDRESS *Ip6
> )
> {
> UINT8 Index;
>
> + ASSERT (Ip6 != NULL);
> +
> for (Index = 0; Index < 16; Index++) {
> if (Ip6->Addr[Index] != 0) {
> return FALSE;
> }
> }
> @@ -741,10 +764,12 @@ NetIp6IsUnspecifiedAddr (
> }
>
> /**
> Check whether the incoming Ipv6 address is a link-local address.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - Yes, link-local address
> @retval FALSE - No
>
> @@ -777,10 +802,13 @@ NetIp6IsLinkLocalAddr (
> }
>
> /**
> Check whether the Ipv6 address1 and address2 are on the connected
> network.
>
> + ASSERT if Ip1 or Ip2 is NULL.
> + ASSERT if PrefixLength exceeds IP6_PREFIX_MAX.
> +
> @param[in] Ip1 - Ip6 address1, in network order.
> @param[in] Ip2 - Ip6 address2, in network order.
> @param[in] PrefixLength - The prefix length of the checking net.
>
> @retval TRUE - Yes, connected.
> @@ -813,11 +841,10 @@ NetIp6IsNetEqual (
> }
>
> if (Bit > 0) {
> Mask = (UINT8) (0xFF << (8 - Bit));
>
> - ASSERT (Byte < 16);
> if ((Ip1->Addr[Byte] & Mask) != (Ip2->Addr[Byte] & Mask)) {
> return FALSE;
> }
> }
>
> @@ -826,10 +853,12 @@ NetIp6IsNetEqual (
>
>
> /**
> Switches the endianess of an IPv6 address
>
> + ASSERT if Ip6 is NULL.
> +
> This function swaps the bytes in a 128-bit IPv6 address to switch the value
> from little endian to big endian or vice versa. The byte swapped value is
> returned.
>
> @param Ip6 Points to an IPv6 address
> @@ -844,10 +873,12 @@ Ip6Swap128 (
> )
> {
> UINT64 High;
> UINT64 Low;
>
> + ASSERT (Ip6 != NULL);
> +
> CopyMem (&High, Ip6, sizeof (UINT64));
> CopyMem (&Low, &Ip6->Addr[8], sizeof (UINT64));
>
> High = SwapBytes64 (High);
> Low = SwapBytes64 (Low);
> @@ -891,10 +922,12 @@ NetRandomInitSeed (
>
>
> /**
> Extract a UINT32 from a byte stream.
>
> + ASSERT if Buf is NULL.
> +
> Copy a UINT32 from a byte stream, then converts it from Network
> byte order to host byte order. Use this function to avoid alignment error.
>
> @param[in] Buf The buffer to extract the UINT32.
>
> @@ -907,18 +940,22 @@ NetGetUint32 (
> IN UINT8 *Buf
> )
> {
> UINT32 Value;
>
> + ASSERT (Buf != NULL);
> +
> CopyMem (&Value, Buf, sizeof (UINT32));
> return NTOHL (Value);
> }
>
>
> /**
> Put a UINT32 to the byte stream in network byte order.
>
> + ASSERT if Buf is NULL.
> +
> Converts a UINT32 from host byte order to network byte order. Then copy
> it to the
> byte stream.
>
> @param[in, out] Buf The buffer to put the UINT32.
> @param[in] Data The data to be converted and put into the byte
> stream.
> @@ -929,10 +966,12 @@ EFIAPI
> NetPutUint32 (
> IN OUT UINT8 *Buf,
> IN UINT32 Data
> )
> {
> + ASSERT (Buf != NULL);
> +
> Data = HTONL (Data);
> CopyMem (Buf, &Data, sizeof (UINT32));
> }
>
>
> @@ -1027,10 +1066,12 @@ NetListRemoveTail (
>
>
> /**
> Insert a new node entry after a designated node entry of a doubly linked
> list.
>
> + ASSERT if PrevEntry or NewEntry is NULL.
> +
> Inserts a new node entry donated by NewEntry after the node entry
> donated by PrevEntry
> of the doubly linked list.
>
> @param[in, out] PrevEntry The previous entry to insert after.
> @param[in, out] NewEntry The new entry to insert.
> @@ -1041,20 +1082,24 @@ EFIAPI
> NetListInsertAfter (
> IN OUT LIST_ENTRY *PrevEntry,
> IN OUT LIST_ENTRY *NewEntry
> )
> {
> + ASSERT (PrevEntry != NULL && NewEntry != NULL);
> +
> NewEntry->BackLink = PrevEntry;
> NewEntry->ForwardLink = PrevEntry->ForwardLink;
> PrevEntry->ForwardLink->BackLink = NewEntry;
> PrevEntry->ForwardLink = NewEntry;
> }
>
>
> /**
> Insert a new node entry before a designated node entry of a doubly linked
> list.
>
> + ASSERT if PostEntry or NewEntry is NULL.
> +
> Inserts a new node entry donated by NewEntry after the node entry
> donated by PostEntry
> of the doubly linked list.
>
> @param[in, out] PostEntry The entry to insert before.
> @param[in, out] NewEntry The new entry to insert.
> @@ -1065,10 +1110,12 @@ EFIAPI
> NetListInsertBefore (
> IN OUT LIST_ENTRY *PostEntry,
> IN OUT LIST_ENTRY *NewEntry
> )
> {
> + ASSERT (PostEntry != NULL && NewEntry != NULL);
> +
> NewEntry->ForwardLink = PostEntry;
> NewEntry->BackLink = PostEntry->BackLink;
> PostEntry->BackLink->ForwardLink = NewEntry;
> PostEntry->BackLink = NewEntry;
> }
> @@ -1263,11 +1310,10 @@ NetMapClean (
>
> If the number of the <Key, Value> pairs in the netmap is zero, return TRUE.
>
> If Map is NULL, then ASSERT().
>
> -
> @param[in] Map The net map to test.
>
> @return TRUE if the netmap is empty, otherwise FALSE.
>
> **/
> @@ -1283,10 +1329,12 @@ NetMapIsEmpty (
>
>
> /**
> Return the number of the <Key, Value> pairs in the netmap.
>
> + If Map is NULL, then ASSERT().
> +
> @param[in] Map The netmap to get the entry number.
>
> @return The entry number in the netmap.
>
> **/
> @@ -1294,10 +1342,11 @@ UINTN
> EFIAPI
> NetMapGetCount (
> IN NET_MAP *Map
> )
> {
> + ASSERT (Map != NULL);
> return Map->Count;
> }
>
>
> /**
> @@ -1358,10 +1407,11 @@ NetMapAllocItem (
> Allocate an item to save the <Key, Value> pair and add corresponding node
> entry
> to the beginning of the Used doubly linked list. The number of the <Key,
> Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @param[in, out] Map The netmap to insert into.
> @param[in] Key The user's key.
> @param[in] Value The user's value for the key.
>
> @@ -1377,11 +1427,11 @@ NetMapInsertHead (
> IN VOID *Value OPTIONAL
> )
> {
> NET_MAP_ITEM *Item;
>
> - ASSERT (Map != NULL);
> + ASSERT (Map != NULL && Key != NULL);
>
> Item = NetMapAllocItem (Map);
>
> if (Item == NULL) {
> return EFI_OUT_OF_RESOURCES;
> @@ -1402,10 +1452,11 @@ NetMapInsertHead (
> Allocate an item to save the <Key, Value> pair and add corresponding node
> entry
> to the tail of the Used doubly linked list. The number of the <Key, Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @param[in, out] Map The netmap to insert into.
> @param[in] Key The user's key.
> @param[in] Value The user's value for the key.
>
> @@ -1421,11 +1472,11 @@ NetMapInsertTail (
> IN VOID *Value OPTIONAL
> )
> {
> NET_MAP_ITEM *Item;
>
> - ASSERT (Map != NULL);
> + ASSERT (Map != NULL && Key != NULL);
>
> Item = NetMapAllocItem (Map);
>
> if (Item == NULL) {
> return EFI_OUT_OF_RESOURCES;
> @@ -1442,10 +1493,13 @@ NetMapInsertTail (
>
>
> /**
> Check whether the item is in the Map and return TRUE if it is.
>
> + If Map is NULL, then ASSERT().
> + If Item is NULL, then ASSERT().
> +
> @param[in] Map The netmap to search within.
> @param[in] Item The item to search.
>
> @return TRUE if the item is in the netmap, otherwise FALSE.
>
> @@ -1456,10 +1510,12 @@ NetItemInMap (
> IN NET_MAP_ITEM *Item
> )
> {
> LIST_ENTRY *ListEntry;
>
> + ASSERT (Map != NULL && Item != NULL);
> +
> NET_LIST_FOR_EACH (ListEntry, &Map->Used) {
> if (ListEntry == &Item->Link) {
> return TRUE;
> }
> }
> @@ -1473,10 +1529,11 @@ NetItemInMap (
>
> Iterate the Used doubly linked list of the netmap to get every item.
> Compare the key of every
> item with the key to search. It returns the point to the item contains the
> Key if found.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @param[in] Map The netmap to search within.
> @param[in] Key The key to search.
>
> @return The point to the item contains the Key, or NULL if Key isn't in the
> map.
> @@ -1490,11 +1547,11 @@ NetMapFindKey (
> )
> {
> LIST_ENTRY *Entry;
> NET_MAP_ITEM *Item;
>
> - ASSERT (Map != NULL);
> + ASSERT (Map != NULL && Key != NULL);
>
> NET_LIST_FOR_EACH (Entry, &Map->Used) {
> Item = NET_LIST_USER_STRUCT (Entry, NET_MAP_ITEM, Link);
>
> if (Item->Key == Key) {
> @@ -2093,10 +2150,13 @@ NetLibGetVlanHandle (
> }
>
> /**
> Get MAC address associated with the network service handle.
>
> + If MacAddress is NULL, then ASSERT().
> + If AddressSize is NULL, then ASSERT().
> +
> There should be MNP Service Binding Protocol installed on the input
> ServiceHandle.
> If SNP is installed on the ServiceHandle or its parent handle, MAC address
> will
> be retrieved from SNP. If no SNP found, try to get SNP mode data use MNP.
>
> @param[in] ServiceHandle The handle where network service binding
> protocols are
> @@ -2197,10 +2257,12 @@ NetLibGetMacAddress (
>
> /**
> Convert MAC address of the NIC associated with specified Service Binding
> Handle
> to a unicode string. Callers are responsible for freeing the string storage.
>
> + If MacString is NULL, then ASSERT().
> +
> Locate simple network protocol associated with the Service Binding Handle
> and
> get the mac address from SNP. Then convert the mac address into a
> unicode
> string. It takes 2 unicode characters to represent a 1 byte binary buffer.
> Plus one unicode character for the null-terminator.
>
> @@ -2296,10 +2358,12 @@ NetLibGetMacString (
> }
>
> /**
> Detect media status for specified network device.
>
> + If MediaPresent is NULL, then ASSERT().
> +
> The underlying UNDI driver may or may not support reporting media status
> from
> GET_STATUS command
> (PXE_STATFLAGS_GET_STATUS_NO_MEDIA_SUPPORTED). This routine
> will try to invoke Snp->GetStatus() to get the media status: if media already
> present, it return directly; if media not present, it will stop SNP and then
> restart SNP to get the latest media status, this give chance to get the
> correct
> @@ -2404,10 +2468,14 @@ NetLibDetectMedia (
> MCastFilter = AllocateCopyPool (
> MCastFilterCount * sizeof (EFI_MAC_ADDRESS),
> Snp->Mode->MCastFilter
> );
> ASSERT (MCastFilter != NULL);
> + if (MCastFilter == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Exit;
> + }
>
> ResetMCastFilters = FALSE;
> }
>
> //
> @@ -2735,10 +2803,12 @@ ON_EXIT:
> }
>
> /**
> Create an IPv4 device path node.
>
> + If Node is NULL, then ASSERT().
> +
> The header type of IPv4 device path node is MESSAGING_DEVICE_PATH.
> The header subtype of IPv4 device path node is MSG_IPv4_DP.
> Get other info from parameters to make up the whole IPv4 device path
> node.
>
> @param[in, out] Node Pointer to the IPv4 device path node.
> @@ -2762,10 +2832,12 @@ NetLibCreateIPv4DPathNode (
> IN UINT16 RemotePort,
> IN UINT16 Protocol,
> IN BOOLEAN UseDefaultAddress
> )
> {
> + ASSERT (Node != NULL);
> +
> Node->Header.Type = MESSAGING_DEVICE_PATH;
> Node->Header.SubType = MSG_IPv4_DP;
> SetDevicePathNodeLength (&Node->Header, sizeof (IPv4_DEVICE_PATH));
>
> CopyMem (&Node->LocalIpAddress, &LocalIp, sizeof (EFI_IPv4_ADDRESS));
> @@ -2792,10 +2864,14 @@ NetLibCreateIPv4DPathNode (
> }
>
> /**
> Create an IPv6 device path node.
>
> + If Node is NULL, then ASSERT().
> + If LocalIp is NULL, then ASSERT().
> + If RemoteIp is NULL, then ASSERT().
> +
> The header type of IPv6 device path node is MESSAGING_DEVICE_PATH.
> The header subtype of IPv6 device path node is MSG_IPv6_DP.
> Get other info from parameters to make up the whole IPv6 device path
> node.
>
> @param[in, out] Node Pointer to the IPv6 device path node.
> @@ -2817,10 +2893,12 @@ NetLibCreateIPv6DPathNode (
> IN EFI_IPv6_ADDRESS *RemoteIp,
> IN UINT16 RemotePort,
> IN UINT16 Protocol
> )
> {
> + ASSERT (Node != NULL && LocalIp != NULL && RemoteIp != NULL);
> +
> Node->Header.Type = MESSAGING_DEVICE_PATH;
> Node->Header.SubType = MSG_IPv6_DP;
> SetDevicePathNodeLength (&Node->Header, sizeof (IPv6_DEVICE_PATH));
>
> CopyMem (&Node->LocalIpAddress, LocalIp, sizeof (EFI_IPv6_ADDRESS));
> @@ -2841,10 +2919,12 @@ NetLibCreateIPv6DPathNode (
> }
>
> /**
> Find the UNDI/SNP handle from controller and protocol GUID.
>
> + If ProtocolGuid is NULL, then ASSERT().
> +
> For example, IP will open a MNP child to transmit/receive
> packets, when MNP is stopped, IP should also be stopped. IP
> needs to find its own private data which is related the IP's
> service binding instance that is install on UNDI/SNP handle.
> Now, the controller is either a MNP or ARP child handle. But
> @@ -2868,10 +2948,12 @@ NetLibGetNicHandle (
> EFI_HANDLE Handle;
> EFI_STATUS Status;
> UINTN OpenCount;
> UINTN Index;
>
> + ASSERT (ProtocolGuid != NULL);
> +
> Status = gBS->OpenProtocolInformation (
> Controller,
> ProtocolGuid,
> &OpenBuffer,
> &OpenCount
> @@ -3149,10 +3231,12 @@ NetLibIp6ToStr (
> }
>
> /**
> This function obtains the system guid from the smbios table.
>
> + If SystemGuid is NULL, then ASSERT().
> +
> @param[out] SystemGuid The pointer of the returned system guid.
>
> @retval EFI_SUCCESS Successfully obtained the system guid.
> @retval EFI_NOT_FOUND Did not find the SMBIOS table.
>
> @@ -3168,10 +3252,12 @@ NetLibGetSystemGuid (
> SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;
> SMBIOS_STRUCTURE_POINTER Smbios;
> SMBIOS_STRUCTURE_POINTER SmbiosEnd;
> CHAR8 *String;
>
> + ASSERT (SystemGuid != NULL);
> +
> SmbiosTable = NULL;
> Status = EfiGetSystemConfigurationTable (&gEfiSmbios3TableGuid, (VOID
> **) &Smbios30Table);
> if (!(EFI_ERROR (Status) || Smbios30Table == NULL)) {
> Smbios.Hdr = (SMBIOS_STRUCTURE *) (UINTN) Smbios30Table-
> >TableAddress;
> SmbiosEnd.Raw = (UINT8 *) (UINTN) (Smbios30Table->TableAddress +
> Smbios30Table->TableMaximumSize);
> @@ -3234,11 +3320,14 @@ NetLibGetSystemGuid (
> } while (Smbios.Raw < SmbiosEnd.Raw);
> return EFI_NOT_FOUND;
> }
>
> /**
> - Create Dns QName according the queried domain name.
> + Create Dns QName according the queried domain name.
> +
> + If DomainName is NULL, then ASSERT().
> +
> QName is a domain name represented as a sequence of labels,
> where each label consists of a length octet followed by that
> number of octets. The QName terminates with the zero
> length octet for the null label of the root. Caller should
> take responsibility to free the buffer in returned pointer.
> @@ -3260,10 +3349,12 @@ NetLibCreateDnsQName (
> CHAR8 *Header;
> CHAR8 *Tail;
> UINTN Len;
> UINTN Index;
>
> + ASSERT (DomainName != NULL);
> +
> QueryName = NULL;
> QueryNameSize = 0;
> Header = NULL;
> Tail = NULL;
>
> --
> 1.9.5.msysgit.1
next prev parent reply other threads:[~2018-01-03 5:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-03 2:31 [Patch 0/3] Fix a set of issues for DxeNetLib fanwang2
2018-01-03 2:31 ` [Patch 1/3] MdeModulePkg/DxeNetLib: Fix a potential issue in macro definition fanwang2
2018-01-03 6:03 ` Wu, Jiaxin
2018-01-03 2:31 ` [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling fanwang2
2018-01-03 6:03 ` Wu, Jiaxin [this message]
2018-01-03 2:31 ` [Patch 3/3] MdeModulePkg/DxeNetLib: Fix an error in packet length counting fanwang2
2018-01-03 6:03 ` Wu, Jiaxin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=895558F6EA4E3B41AC93A00D163B727416358D9F@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox