public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Wang Fan <fan.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Jiaxin Wu <jiaxin.wu@intel.com>, Ye Ting <ting.ye@intel.com>,
	Fu Siyuan <siyuan.fu@intel.com>
Subject: [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
Date: Wed, 10 Jan 2018 11:16:28 +0800	[thread overview]
Message-ID: <1515554188-2560-3-git-send-email-fan.wang@intel.com> (raw)
In-Reply-To: <1515554188-2560-1-git-send-email-fan.wang@intel.com>

* In DxeIpIo, there are several places use ASSERT() to check input
  parameters without and descriptions or error handling. This patch
  fixed this issue.
* Fixed some incorrect descriptions in code commence.
* Remove unneeded Exit tag in function IpIoOpen and IpIoConfigIp.
* Add EFIAPI tag for function IpIoRefreshNeighbor.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
 MdeModulePkg/Include/Library/IpIoLib.h       | 21 +++++++++--
 MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 52 ++++++++++++++++++----------
 2 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Include/Library/IpIoLib.h b/MdeModulePkg/Include/Library/IpIoLib.h
index 463bf95..61653b0 100644
--- a/MdeModulePkg/Include/Library/IpIoLib.h
+++ b/MdeModulePkg/Include/Library/IpIoLib.h
@@ -308,10 +308,12 @@ typedef struct _IP_IO_IP_INFO {
   UINT8                     IpVersion;
 } IP_IO_IP_INFO;
 
 /**
   Create a new IP_IO instance.
+
+  If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function uses IP4/IP6 service binding protocol in Controller to create
   an IP4/IP6 child (aka IP4/IP6 instance).
 
   @param[in]  Image             The image handle of the driver or application that
@@ -351,10 +353,12 @@ IpIoDestroy (
   IN OUT IP_IO *IpIo
   );
 
 /**
   Stop an IP_IO instance.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function is paired with IpIoOpen(). The IP_IO will be unconfigured, and all
   pending send/receive tokens will be canceled.
 
   @param[in, out]  IpIo            The pointer to the IP_IO instance that needs to stop.
@@ -370,11 +374,13 @@ IpIoStop (
   IN OUT IP_IO *IpIo
   );
 
 /**
   Open an IP_IO instance for use.
-  
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   This function is called after IpIoCreate(). It is used for configuring the IP
   instance and register the callbacks and their context data for sending and
   receiving IP packets.
 
   @param[in, out]  IpIo               The pointer to an IP_IO instance that needs
@@ -399,11 +405,11 @@ IpIoOpen (
   );
 
 /**
   Send out an IP packet.
   
-  This function is called after IpIoOpen(). The data to be sent are wrapped in
+  This function is called after IpIoOpen(). The data to be sent is wrapped in
   Pkt. The IP instance wrapped in IpIo is used for sending by default but can be
   overriden by Sender. Other sending configs, like source address and gateway
   address etc., are specified in OverrideData.
 
   @param[in, out]  IpIo                  Pointer to an IP_IO instance used for sending IP
@@ -437,10 +443,13 @@ IpIoSend (
   );
 
 /**
   Cancel the IP transmit token that wraps this Packet.
 
+  If IpIo is NULL, then ASSERT().
+  If Packet is NULL, then ASSERT().
+
   @param[in]  IpIo                  The pointer to the IP_IO instance.
   @param[in]  Packet                The pointer to the packet of NET_BUF to cancel.
 
 **/
 VOID
@@ -450,10 +459,13 @@ IpIoCancelTxToken (
   IN VOID   *Packet
   );
 
 /**
   Add a new IP instance for sending data.
+
+  If IpIo is NULL, then ASSERT().
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   The function is used to add the IP_IO to the IP_IO sending list. The caller
   can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send
   data.
 
@@ -471,10 +483,12 @@ IpIoAddIp (
 
 /**
   Configure the IP instance of this IpInfo and start the receiving if IpConfigData
   is not NULL.
 
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   @param[in, out]  IpInfo          The pointer to the IP_IO_IP_INFO instance.
   @param[in, out]  IpConfigData    The IP4 or IP6 configure data used to configure 
                                    the IP instance. If NULL, the IP instance is reset.
                                    If UseDefaultAddress is set to TRUE, and the configure
                                    operation succeeds, the default address information
@@ -493,10 +507,12 @@ IpIoConfigIp (
   );
 
 /**
   Destroy an IP instance maintained in IpIo->IpList for
   sending purpose.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().  
   
   This function pairs with IpIoAddIp(). The IpInfo is previously created by
   IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance
   will be dstroyed if the RefCnt is zero.
 
@@ -584,10 +600,11 @@ IpIoGetIcmpErrStatus (
   @retval      EFI_UNSUPPORTED       IP version is IPv4, which doesn't support neighbor cache refresh.
   @retval      EFI_OUT_OF_RESOURCES  Failed due to resource limitations.
 
 **/
 EFI_STATUS
+EFIAPI
 IpIoRefreshNeighbor (
   IN IP_IO           *IpIo,
   IN EFI_IP_ADDRESS  *Neighbor,
   IN UINT32          Timeout  
   );
diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
index c7bc1aa..0c6681d 100644
--- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
+++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
@@ -1,10 +1,10 @@
 /** @file
   IpIo Library.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -632,12 +632,12 @@ IpIoExtFree (
   @param[in]       Context              Pointer to the context.
   @param[in]       NotifyData           Pointer to the notify data.
   @param[in]       Dest                 Pointer to the destination IP address.
   @param[in]       Override             Pointer to the overriden IP_IO data.
 
-  @return Pointer to the data structure created to wrap the packet. If NULL,
-  @return resource limit occurred.
+  @return Pointer to the data structure created to wrap the packet. If any error occurs, 
+          then return NULL.
 
 **/
 IP_IO_SEND_ENTRY *
 IpIoCreateSndEntry (
   IN OUT IP_IO             *IpIo,
@@ -1073,11 +1073,11 @@ IpIoListenHandlerDpc (
     if ((EFI_IP4 (RxData->Ip4RxData.Header->SourceAddress) != 0) &&
         (IpIo->SubnetMask != 0) &&
         IP4_NET_EQUAL (IpIo->StationIp, EFI_NTOHL (((EFI_IP4_RECEIVE_DATA *) RxData)->Header->SourceAddress), IpIo->SubnetMask) &&
         !NetIp4IsUnicast (EFI_NTOHL (((EFI_IP4_RECEIVE_DATA *) RxData)->Header->SourceAddress), IpIo->SubnetMask)) {
       //
-      // The source address is not zero and it's not a unicast IP address, discard it.
+      // The source address doesn't match StationIp and it's not a unicast IP address, discard it.
       //
       goto CleanUp;
     }
 
     if (RxData->Ip4RxData.DataLength == 0) {
@@ -1219,10 +1219,12 @@ IpIoListenHandler (
 }
 
 
 /**
   Create a new IP_IO instance.
+
+  If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function uses IP4/IP6 service binding protocol in Controller to create
   an IP4/IP6 child (aka IP4/IP6 instance).
 
   @param[in]  Image             The image handle of the driver or application that
@@ -1284,11 +1286,11 @@ IpIoCreate (
   Status = IpIoCreateIpChildOpenProtocol (
              Controller,
              Image,
              &IpIo->ChildHandle,
              IpVersion,             
-             (VOID **)&(IpIo->Ip)
+             (VOID **) & (IpIo->Ip)
              );
   if (EFI_ERROR (Status)) {
     goto ReleaseIpIo;
   }
 
@@ -1306,11 +1308,13 @@ ReleaseIpIo:
 }
 
 
 /**
   Open an IP_IO instance for use.
-  
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   This function is called after IpIoCreate(). It is used for configuring the IP
   instance and register the callbacks and their context data for sending and
   receiving IP packets.
 
   @param[in, out]  IpIo               Pointer to an IP_IO instance that needs
@@ -1417,11 +1421,11 @@ IpIoOpen (
                              IpIo->Ip.Ip4,
                              &(IpIo->RcvToken.Ip4Token)
                              );
     if (EFI_ERROR (Status)) {
       IpIo->Ip.Ip4->Configure (IpIo->Ip.Ip4, NULL);
-      goto ErrorExit;
+      return Status;
     }
 
   } else {
 
     IpIo->Protocol = OpenData->IpConfigData.Ip6CfgData.DefaultProtocol;
@@ -1429,25 +1433,25 @@ IpIoOpen (
                              IpIo->Ip.Ip6,
                              &(IpIo->RcvToken.Ip6Token)
                              );
     if (EFI_ERROR (Status)) {
       IpIo->Ip.Ip6->Configure (IpIo->Ip.Ip6, NULL);
-      goto ErrorExit;
+      return Status;
     }
   }
 
   IpIo->IsConfigured = TRUE;
   InsertTailList (&mActiveIpIoList, &IpIo->Entry);
 
-ErrorExit:
-
   return Status;
 }
 
 
 /**
   Stop an IP_IO instance.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function is paired with IpIoOpen(). The IP_IO will be unconfigured and all
   the pending send/receive tokens will be canceled.
 
   @param[in, out]  IpIo            Pointer to the IP_IO instance that needs to stop.
@@ -1575,11 +1579,11 @@ IpIoDestroy (
 
 
 /**
   Send out an IP packet.
   
-  This function is called after IpIoOpen(). The data to be sent are wrapped in
+  This function is called after IpIoOpen(). The data to be sent is wrapped in
   Pkt. The IP instance wrapped in IpIo is used for sending by default but can be
   overriden by Sender. Other sending configs, like source address and gateway
   address etc., are specified in OverrideData.
 
   @param[in, out]  IpIo                  Pointer to an IP_IO instance used for sending IP
@@ -1662,10 +1666,13 @@ IpIoSend (
 
 
 /**
   Cancel the IP transmit token which wraps this Packet.
 
+  If IpIo is NULL, then ASSERT().
+  If Packet is NULL, then ASSERT().
+
   @param[in]  IpIo                  Pointer to the IP_IO instance.
   @param[in]  Packet                Pointer to the packet of NET_BUF to cancel.
 
 **/
 VOID
@@ -1708,10 +1715,13 @@ IpIoCancelTxToken (
 }
 
 
 /**
   Add a new IP instance for sending data.
+
+  If IpIo is NULL, then ASSERT().
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   The function is used to add the IP_IO to the IP_IO sending list. The caller
   can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send
   data.
 
@@ -1735,10 +1745,11 @@ IpIoAddIp (
 
   IpInfo = AllocatePool (sizeof (IP_IO_IP_INFO));
   if (IpInfo == NULL) {
     return NULL;
   }
+  ASSERT ((IpInfo->IpVersion == IP_VERSION_4) || (IpInfo->IpVersion == IP_VERSION_6));
 
   //
   // Init this IpInfo, set the Addr and SubnetMask to 0 before we configure the IP
   // instance.
   //
@@ -1810,10 +1821,13 @@ ReleaseIpInfo:
 
 /**
   Configure the IP instance of this IpInfo and start the receiving if IpConfigData
   is not NULL.
 
+  If IpInfo is NULL, then ASSERT().
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   @param[in, out]  IpInfo          Pointer to the IP_IO_IP_INFO instance.
   @param[in, out]  IpConfigData    The IP configure data used to configure the IP
                                    instance, if NULL the IP instance is reset. If
                                    UseDefaultAddress is set to TRUE, and the configure
                                    operation succeeds, the default address information
@@ -1859,11 +1873,11 @@ IpIoConfigIp (
   } else {
     Status = Ip.Ip6->Configure (Ip.Ip6, IpConfigData);
   }
 
   if (EFI_ERROR (Status)) {
-    goto OnExit;
+    return Status;
   }
 
   if (IpConfigData != NULL) {
     if (IpInfo->IpVersion == IP_VERSION_4) {
 
@@ -1874,11 +1888,11 @@ IpIoConfigIp (
                            NULL, 
                            NULL
                            );
         if (EFI_ERROR (Status)) {
           Ip.Ip4->Configure (Ip.Ip4, NULL);
-          goto OnExit;
+          return Status;
         }
 
         IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)->StationAddress, &Ip4ModeData.ConfigData.StationAddress);
         IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)->SubnetMask, &Ip4ModeData.ConfigData.SubnetMask);
       }
@@ -1908,11 +1922,11 @@ IpIoConfigIp (
                          NULL,
                          NULL
                          );
       if (EFI_ERROR (Status)) {
         Ip.Ip6->Configure (Ip.Ip6, NULL);
-        goto OnExit;
+        return Status;
       }
 
       if (Ip6ModeData.IsConfigured) {
         CopyMem (
           &((EFI_IP6_CONFIG_DATA *) IpConfigData)->StationAddress,
@@ -1944,11 +1958,11 @@ IpIoConfigIp (
           FreePool (Ip6ModeData.IcmpTypeList);
         }
 
       } else {
         Status = EFI_NO_MAPPING;
-        goto OnExit;
+        return Status;
       } 
 
       CopyMem (
         &IpInfo->Addr, 
         &Ip6ModeData.ConfigData.StationAddress, 
@@ -1969,19 +1983,19 @@ IpIoConfigIp (
     //
     ZeroMem (&IpInfo->Addr, sizeof (IpInfo->Addr));
     ZeroMem (&IpInfo->PreMask, sizeof (IpInfo->PreMask));
   }
 
-OnExit:
-
   return Status;
 }
 
 
 /**
   Destroy an IP instance maintained in IpIo->IpList for
   sending purpose.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function pairs with IpIoAddIp(). The IpInfo is previously created by
   IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance
   will be dstroyed if the RefCnt is zero.
 
@@ -2110,12 +2124,11 @@ IpIoFindSender (
 
         if (EFI_IP6_EQUAL (&IpInfo->Addr.v6, &Src->v6)) {
           *IpIo = IpIoPtr;
           return IpInfo;       
         }
-      }      
-
+      }
     }
   }
 
   //
   // No match.
@@ -2258,10 +2271,11 @@ IpIoGetIcmpErrStatus (
   @retval      EFI_UNSUPPORTED       IP version is IPv4, which doesn't support neighbor cache refresh.
   @retval      EFI_OUT_OF_RESOURCES  Failed due to resource limit.
 
 **/
 EFI_STATUS
+EFIAPI
 IpIoRefreshNeighbor (
   IN IP_IO           *IpIo,
   IN EFI_IP_ADDRESS  *Neighbor,
   IN UINT32          Timeout  
   )
-- 
1.9.5.msysgit.1



  parent reply	other threads:[~2018-01-10  3:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10  3:16 [Patch 0/2] Fixed some issues in DxeIpIoLib Wang Fan
2018-01-10  3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
2018-01-15  5:46   ` Fu, Siyuan
2018-01-16  0:44   ` Wu, Jiaxin
2018-01-10  3:16 ` Wang Fan [this message]
2018-01-10  4:51   ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Ni, Ruiyu
2018-01-10  4:58     ` Wang, Fan
2018-01-15  5:48   ` Fu, Siyuan

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=1515554188-2560-3-git-send-email-fan.wang@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