[PATCH 0/5] NetworkPkg: Remove the redundant code and definition.

[PATCH 0/5] NetworkPkg: Remove the redundant code and definition.

[Songpeng Li]

Some redundant functions and definitions which are never calld have been 
removed. These fuctions are manually searched in source files to make
sure that they are not used elsewhere.

Songpeng Li (5):
  NetworkPkg: IScsiDxe: Remove the redundant code and definition.
  NetworkPkg: IpSecDxe: Remove the redundant code.
  NetworkPkg: TcpDxe: Remove the redundant code.
  NetworkPkg: TlsDxe: Remove the redundant definition.
  NetworkPkg: UefiPxeBcDxe: Remove the redundant code.

 NetworkPkg/IScsiDxe/IScsiDxe.inf     |  1 -
 NetworkPkg/IScsiDxe/IScsiProto.c     | 33 ------------
 NetworkPkg/IpSecDxe/Ikev2/Payload.c  | 18 -------
 NetworkPkg/IpSecDxe/Ikev2/Utility.c  | 76 ----------------------------
 NetworkPkg/IpSecDxe/Ikev2/Utility.h  | 67 ------------------------
 NetworkPkg/TcpDxe/TcpOption.c        | 32 +-----------
 NetworkPkg/TcpDxe/TcpOption.h        | 18 +------
 NetworkPkg/TlsDxe/TlsDxe.inf         |  3 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 23 ---------
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 10 ----
 10 files changed, 3 insertions(+), 278 deletions(-)

-- 
2.18.0.windows.1

[PATCH 1/5] NetworkPkg: IScsiDxe: Remove the redundant code and definition.

[Songpeng Li]

The function IScsiFindTcbByITT that is never called have been removed.
The definition gEfiAcpi20TableGuid has been removed.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Songpeng Li <songpeng.li@intel.com>
---
 NetworkPkg/IScsiDxe/IScsiDxe.inf |  1 -
 NetworkPkg/IScsiDxe/IScsiProto.c | 33 --------------------------------
 2 files changed, 34 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
index 995ec853d2..61a0693a07 100644
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
@@ -124,7 +124,6 @@
   gEfiIfrTianoGuid                              ## SOMETIMES_PRODUCES ## UNDEFINED
   gEfiAcpiTableGuid                             ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES ## SystemTable
-  gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAdapterInfoNetworkBootGuid                ## SOMETIMES_CONSUMES ## UNDEFINED
   gEfiAdapterInfoUndiIpv6SupportGuid            ## SOMETIMES_CONSUMES ## GUID
 
diff --git a/NetworkPkg/IScsiDxe/IScsiProto.c b/NetworkPkg/IScsiDxe/IScsiProto.c
index 7619360568..f4a49c677a 100644
--- a/NetworkPkg/IScsiDxe/IScsiProto.c
+++ b/NetworkPkg/IScsiDxe/IScsiProto.c
@@ -2096,39 +2096,6 @@ IScsiDelTcb (
 }
 
 
-/**
-  Find the task control block by the initator task tag.
-
-  @param[in]  TcbList         The tcb list.
-  @param[in]  InitiatorTaskTag The initiator task tag.
-
-  @return The task control block found.
-  @retval NULL The task control block cannot be found.
-
-**/
-ISCSI_TCB *
-IScsiFindTcbByITT (
-  IN LIST_ENTRY      *TcbList,
-  IN UINT32          InitiatorTaskTag
-  )
-{
-  ISCSI_TCB       *Tcb;
-  LIST_ENTRY      *Entry;
-
-  Tcb = NULL;
-
-  NET_LIST_FOR_EACH (Entry, TcbList) {
-    Tcb = NET_LIST_USER_STRUCT (Entry, ISCSI_TCB, Link);
-
-    if (Tcb->InitiatorTaskTag == InitiatorTaskTag) {
-      break;
-    }
-  }
-
-  return Tcb;
-}
-
-
 /**
   Create a data segment, pad it, and calculate the CRC if needed.
 
-- 
2.18.0.windows.1

Re: [PATCH 1/5] NetworkPkg: IScsiDxe: Remove the redundant code and definition.

[Laszlo Ersek]

On 08/16/18 03:37, Songpeng Li wrote:
> The function IScsiFindTcbByITT that is never called have been removed.
> The definition gEfiAcpi20TableGuid has been removed.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li <songpeng.li@intel.com>
> ---
>  NetworkPkg/IScsiDxe/IScsiDxe.inf |  1 -
>  NetworkPkg/IScsiDxe/IScsiProto.c | 33 --------------------------------
>  2 files changed, 34 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

[PATCH 2/5] NetworkPkg: IpSecDxe: Remove the redundant code.

[Songpeng Li]

The functions that are never called have been removed.
They are Ikev2OnPacketAccepted, Ikev2SaSessionOnDeleting,
Ikev2ChildSaSessionLookupByMid, Ikev2ChildSaSessionOnDeleting
and Ikev2ChildExchangeRekeySpi.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Songpeng Li <songpeng.li@intel.com>
---
 NetworkPkg/IpSecDxe/Ikev2/Payload.c | 18 -------
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 76 -----------------------------
 NetworkPkg/IpSecDxe/Ikev2/Utility.h | 67 -------------------------
 3 files changed, 161 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Payload.c b/NetworkPkg/IpSecDxe/Ikev2/Payload.c
index 218c26f934..1bb5e2e5e5 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Payload.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Payload.c
@@ -3104,24 +3104,6 @@ ON_EXIT:
   return Status;
 }
 
-/**
-  Save some useful payloads after accepting the Packet.
-
-  @param[in] SessionCommon   Pointer to IKEV2_SESSION_COMMON related to the operation.
-  @param[in] IkePacket       Pointer to received IkePacet.
-  @param[in] IkeType         The type used to indicate it is in IkeSa or ChildSa or Info
-                             exchange.
-
-**/
-VOID
-Ikev2OnPacketAccepted (
-  IN IKEV2_SESSION_COMMON *SessionCommon,
-  IN IKE_PACKET           *IkePacket,
-  IN UINT8                IkeType
-  )
-{
-  return;
-}
 
 /**
 
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 698aba1327..0c9c929705 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -290,21 +290,6 @@ Ikev2SaSessionRemove (
   return NULL;
 }
 
-/**
-  Marking a SA session as on deleting.
-
-  @param[in]  IkeSaSession  Pointer to IKEV2_SA_SESSION.
-
-  @retval     EFI_SUCCESS   Find the related SA session and marked it.
-
-**/
-EFI_STATUS
-Ikev2SaSessionOnDeleting (
-  IN IKEV2_SA_SESSION          *IkeSaSession
-  )
-{
-  return EFI_SUCCESS;
-}
 
 /**
   Free specified Seession Common. The session common would belong to a IKE SA or
@@ -659,33 +644,6 @@ Ikev2ChildSaSessionReg (
   return ;
 }
 
-/**
-  Find the ChildSaSession by it's MessagId.
-
-  @param[in] SaSessionList  Pointer to a ChildSaSession List.
-  @param[in] Mid            The messageId used to search ChildSaSession.
-
-  @return Pointer to IKEV2_CHILD_SA_SESSION or NULL.
-
-**/
-IKEV2_CHILD_SA_SESSION *
-Ikev2ChildSaSessionLookupByMid (
-  IN LIST_ENTRY           *SaSessionList,
-  IN UINT32               Mid
-  )
-{
-  LIST_ENTRY              *Entry;
-  IKEV2_CHILD_SA_SESSION  *ChildSaSession;
-
-  NET_LIST_FOR_EACH (Entry, SaSessionList) {
-    ChildSaSession  = IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
-
-    if (ChildSaSession->MessageId == Mid) {
-      return ChildSaSession;
-    }
-  }
-  return NULL;
-}
 
 /**
   This function find the Child SA by the specified SPI.
@@ -776,22 +734,6 @@ Ikev2ChildSaSessionRemove (
   return NULL;
 }
 
-/**
-  Mark a specified Child SA Session as on deleting.
-
-  @param[in]  ChildSaSession   Pointer to IKEV2_CHILD_SA_SESSION.
-
-  @retval     EFI_SUCCESS      Operation is successful.
-
-**/
-EFI_STATUS
-Ikev2ChildSaSessionOnDeleting (
-  IN IKEV2_CHILD_SA_SESSION   *ChildSaSession
-  )
-{
-  return EFI_SUCCESS;
-}
-
 /**
   Free the memory located for the specified IKEV2_CHILD_SA_SESSION.
 
@@ -1137,24 +1079,6 @@ Ikev2ChildSaAssociateSpdEntry (
 }
 
 
-/**
-  This function finds the SPI from Create Child SA Exchange Packet.
-
-  @param[in] IkePacket       Pointer to IKE_PACKET to be searched.
-
-  @retval SPI number or 0 if it is not supported.
-
-**/
-UINT32
-Ikev2ChildExchangeRekeySpi (
-  IN IKE_PACKET               *IkePacket
-  )
-{
-  //
-  // Not support yet.
-  //
-  return 0;
-}
 
 /**
   Validate the IKE header of received IKE packet.
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.h b/NetworkPkg/IpSecDxe/Ikev2/Utility.h
index 8383ff8dd6..79def7740d 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.h
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.h
@@ -104,19 +104,6 @@ Ikev2SaSessionRemove (
   );
 
 
-/**
-  Marking a SA session as on deleting.
-
-  @param[in]  IkeSaSession  Pointer to IKEV2_SA_SESSION.
-
-  @retval     EFI_SUCCESS   Find the related SA session and marked it.
-
-**/
-EFI_STATUS
-Ikev2SaSessionOnDeleting (
-  IN IKEV2_SA_SESSION          *IkeSaSession
-  );
-
 /**
   After IKE/Child SA is estiblished, close the time event and free sent packet.
 
@@ -211,20 +198,6 @@ Ikev2ChildSaSessionLookupBySpi (
   IN UINT32               Spi
   );
 
-/**
-  Find the ChildSaSession by it's MessagId.
-
-  @param[in] SaSessionList  Pointer to a ChildSaSession List.
-  @param[in] Mid            The messageId used to search ChildSaSession.
-
-  @return Pointer to IKEV2_CHILD_SA_SESSION.
-
-**/
-IKEV2_CHILD_SA_SESSION *
-Ikev2ChildSaSessionLookupByMid (
-  IN LIST_ENTRY           *SaSessionList,
-  IN UINT32               Mid
-  );
 
 /**
   Insert a Child SA Session into the specified ChildSa list..
@@ -257,18 +230,6 @@ Ikev2ChildSaSessionRemove (
   IN UINT8                ListType
   );
 
-/**
-  Mark a specified Child SA Session as on deleting.
-
-  @param[in]  ChildSaSession   Pointer to IKEV2_CHILD_SA_SESSION.
-
-  @retval     EFI_SUCCESS      Operation is successful.
-
-**/
-EFI_STATUS
-Ikev2ChildSaSessionOnDeleting (
-  IN IKEV2_CHILD_SA_SESSION   *ChildSaSession
-  );
 
 /**
   Free the memory located for the specified IKEV2_CHILD_SA_SESSION.
@@ -328,19 +289,6 @@ Ikev2ChildExchangeRequestType(
   IN IKE_PACKET               *IkePacket
   );
 
-/**
-  This function finds the SPI from Create Child Sa Exchange Packet.
-
-  @param[in] IkePacket       Pointer to IKE_PACKET to be searched.
-
-  @retval SPI number.
-
-**/
-UINT32
-Ikev2ChildExchangeRekeySpi(
-  IN IKE_PACKET               *IkePacket
-  );
-
 
 /**
   Associate a SPD selector to the Child SA Session.
@@ -882,21 +830,6 @@ Ikev2DecodePacket (
   IN     UINTN                 IkeType
   );
 
-/**
-  Save some useful payloads after accepting the Packet.
-
-  @param[in] SessionCommon   Pointer to IKEV2_SESSION_COMMON related to the operation.
-  @param[in] IkePacket       Pointer to received IkePacet.
-  @param[in] IkeType         The type used to indicate it is in IkeSa or ChildSa or Info
-                             exchange.
-
-**/
-VOID
-Ikev2OnPacketAccepted (
-  IN IKEV2_SESSION_COMMON *SessionCommon,
-  IN IKE_PACKET           *IkePacket,
-  IN UINT8                IkeType
-  );
 
 /**
   Send out IKEV2 packet.
-- 
2.18.0.windows.1

[PATCH 3/5] NetworkPkg: TcpDxe: Remove the redundant code.

[Songpeng Li]

The function TcpPawsOK that is never called have been removed.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Songpeng Li <songpeng.li@intel.com>
---
 NetworkPkg/TcpDxe/TcpOption.c | 32 +-------------------------------
 NetworkPkg/TcpDxe/TcpOption.h | 18 +-----------------
 2 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpOption.c b/NetworkPkg/TcpDxe/TcpOption.c
index bacce1070d..bd1c2b9c27 100644
--- a/NetworkPkg/TcpDxe/TcpOption.c
+++ b/NetworkPkg/TcpDxe/TcpOption.c
@@ -1,7 +1,7 @@
 /** @file
   Routines to process TCP option.
 
-  Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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
@@ -342,33 +342,3 @@ TcpParseOption (
 
   return 0;
 }
-
-/**
-  Check the segment against PAWS.
-
-  @param[in]  Tcb     Pointer to the TCP_CB of this TCP instance.
-  @param[in]  TSVal   The timestamp value.
-
-  @retval     1       The segment passed the PAWS check.
-  @retval     0       The segment failed to pass the PAWS check.
-
-**/
-UINT32
-TcpPawsOK (
-  IN TCP_CB *Tcb,
-  IN UINT32 TSVal
-  )
-{
-  //
-  // PAWS as defined in RFC1323, buggy...
-  //
-  if (TCP_TIME_LT (TSVal, Tcb->TsRecent) &&
-      TCP_TIME_LT (Tcb->TsRecentAge + TCP_PAWS_24DAY, mTcpTick)
-      ) {
-
-    return 0;
-
-  }
-
-  return 1;
-}
diff --git a/NetworkPkg/TcpDxe/TcpOption.h b/NetworkPkg/TcpDxe/TcpOption.h
index 0ccadb9536..e50fb9b129 100644
--- a/NetworkPkg/TcpDxe/TcpOption.h
+++ b/NetworkPkg/TcpDxe/TcpOption.h
@@ -1,7 +1,7 @@
 /** @file
   Tcp option's routine header file.
 
-  Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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
@@ -126,20 +126,4 @@ TcpParseOption (
   IN OUT TCP_OPTION *Option
   );
 
-/**
-  Check the segment against PAWS.
-
-  @param[in]  Tcb     Pointer to the TCP_CB of this TCP instance.
-  @param[in]  TSVal   The timestamp value.
-
-  @retval     1       The segment passed the PAWS check.
-  @retval     0       The segment failed to pass the PAWS check.
-
-**/
-UINT32
-TcpPawsOK (
-  IN TCP_CB *Tcb,
-  IN UINT32 TSVal
-  );
-
 #endif
-- 
2.18.0.windows.1

Re: [PATCH 3/5] NetworkPkg: TcpDxe: Remove the redundant code.

[Laszlo Ersek]

On 08/16/18 03:37, Songpeng Li wrote:
> The function TcpPawsOK that is never called have been removed.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li <songpeng.li@intel.com>
> ---
>  NetworkPkg/TcpDxe/TcpOption.c | 32 +-------------------------------
>  NetworkPkg/TcpDxe/TcpOption.h | 18 +-----------------
>  2 files changed, 2 insertions(+), 48 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

[PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant definition.

[Songpeng Li]

The definition NetLib has been removed.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Songpeng Li <songpeng.li@intel.com>
---
 NetworkPkg/TlsDxe/TlsDxe.inf | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/NetworkPkg/TlsDxe/TlsDxe.inf b/NetworkPkg/TlsDxe/TlsDxe.inf
index 907feb735b..aaea0fc2ff 100644
--- a/NetworkPkg/TlsDxe/TlsDxe.inf
+++ b/NetworkPkg/TlsDxe/TlsDxe.inf
@@ -5,7 +5,7 @@
 #  This module produces EFI TLS (Transport Layer Security) Protocol and EFI TLS
 #  Service Binding Protocol, to provide TLS services.
 #
-#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 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
@@ -52,7 +52,6 @@
   BaseLib
   UefiLib
   DebugLib
-  NetLib
   BaseCryptLib
   TlsLib
 
-- 
2.18.0.windows.1

Re: [PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant definition.

[Laszlo Ersek]

On 08/16/18 03:37, Songpeng Li wrote:
> The definition NetLib has been removed.

(1) I suggest calling this "lib class dependency", not "definition".

(I could have made the same for a GUID removed in a previous patch, but
there "definition" seemed acceptable. "GUID dependency" would be an
improvement there as well.)

The subject line could also say, "remove superfluous dependencies".

> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li <songpeng.li@intel.com>
> ---
>  NetworkPkg/TlsDxe/TlsDxe.inf | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/TlsDxe/TlsDxe.inf b/NetworkPkg/TlsDxe/TlsDxe.inf
> index 907feb735b..aaea0fc2ff 100644
> --- a/NetworkPkg/TlsDxe/TlsDxe.inf
> +++ b/NetworkPkg/TlsDxe/TlsDxe.inf
> @@ -5,7 +5,7 @@
>  #  This module produces EFI TLS (Transport Layer Security) Protocol and EFI TLS
>  #  Service Binding Protocol, to provide TLS services.
>  #
> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2016 - 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
> @@ -52,7 +52,6 @@
>    BaseLib
>    UefiLib
>    DebugLib
> -  NetLib
>    BaseCryptLib
>    TlsLib
>  
> 

(2) This patch is incomplete; the file "NetworkPkg/TlsDxe/TlsImpl.h"
contains:

#include <Library/NetLib.h>

That header file is huge, so I won't try to audit, on a source code
level, whether TlsDxe makes any NetLib API calls.

(Note that even if it does, then the current patch need not necessarily
cause a linking failure. Because, a NetLib dependency could be inherited
through one of the library instances with which a given platform
resolves the *other* lib class dependencies for TlsDxe.)

So, please remove the #include too, and if TlsDxe still compiles, please
resubmit the patch. (Otherwise, drop the patch, of course.)

Thanks!
Laszlo

Re: [PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant definition.

[Li, Songpeng]

The file "NetworkPkg/TlsDxe/TlsProtocol.c" which contains "NetworkPkg/TlsDxe/TlsImpl.h"
is using the definition HTONS which defined in <Library/NetLib.h>
TlsProtocol.c:
Line 264      *((UINT16 *) Data) = HTONS (TlsGetVersion (Instance->TlsConn));
NetLib.h:
Line 230      #define HTONS(x)   NTOHS(x)

It cannot be removed. Please drop this patch.


Thanks & Best Regards,
Songpeng


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 17, 2018 3:27 AM
> To: Li, Songpeng <songpeng.li@intel.com>; edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: Re: [edk2] [PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant
> definition.
> 
> On 08/16/18 03:37, Songpeng Li wrote:
> > The definition NetLib has been removed.
> 
> (1) I suggest calling this "lib class dependency", not "definition".
> 
> (I could have made the same for a GUID removed in a previous patch, but
> there "definition" seemed acceptable. "GUID dependency" would be an
> improvement there as well.)
> 
> The subject line could also say, "remove superfluous dependencies".
> 
> >
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Songpeng Li <songpeng.li@intel.com>
> > ---
> >  NetworkPkg/TlsDxe/TlsDxe.inf | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/NetworkPkg/TlsDxe/TlsDxe.inf b/NetworkPkg/TlsDxe/TlsDxe.inf
> > index 907feb735b..aaea0fc2ff 100644
> > --- a/NetworkPkg/TlsDxe/TlsDxe.inf
> > +++ b/NetworkPkg/TlsDxe/TlsDxe.inf
> > @@ -5,7 +5,7 @@
> >  #  This module produces EFI TLS (Transport Layer Security) Protocol and EFI
> TLS
> >  #  Service Binding Protocol, to provide TLS services.
> >  #
> > -#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> > +#  Copyright (c) 2016 - 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
> > @@ -52,7 +52,6 @@
> >    BaseLib
> >    UefiLib
> >    DebugLib
> > -  NetLib
> >    BaseCryptLib
> >    TlsLib
> >
> >
> 
> (2) This patch is incomplete; the file "NetworkPkg/TlsDxe/TlsImpl.h"
> contains:
> 
> #include <Library/NetLib.h>
> 
> That header file is huge, so I won't try to audit, on a source code
> level, whether TlsDxe makes any NetLib API calls.
> 
> (Note that even if it does, then the current patch need not necessarily
> cause a linking failure. Because, a NetLib dependency could be inherited
> through one of the library instances with which a given platform
> resolves the *other* lib class dependencies for TlsDxe.)
> 
> So, please remove the #include too, and if TlsDxe still compiles, please
> resubmit the patch. (Otherwise, drop the patch, of course.)
> 
> Thanks!
> Laszlo

Re: [PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant definition.

[Laszlo Ersek]

On 08/17/18 03:36, Li, Songpeng wrote:
> The file "NetworkPkg/TlsDxe/TlsProtocol.c" which contains "NetworkPkg/TlsDxe/TlsImpl.h"
> is using the definition HTONS which defined in <Library/NetLib.h>
> TlsProtocol.c:
> Line 264      *((UINT16 *) Data) = HTONS (TlsGetVersion (Instance->TlsConn));
> NetLib.h:
> Line 230      #define HTONS(x)   NTOHS(x)
> 
> It cannot be removed. Please drop this patch.

Right, I imagined that the code could be using just some macros from the
lib class header, but no extern variables or functions. In such cases, I
prefer to keep the lib class dependency in the INF file as well -- for
example, the macro definition in the lib class header could change at
some point and expand to a call to a new function that is also declared
(as a new API) in the lib class header. In this case, even if all lib
classes were extended with the new API, the client code would no longer
link.

So, as a general rule, including a lib class header implies that the INF
file should specify the lib class dependency too.

I agree that we should drop this patch.

Thanks
Laszlo

> 
> 
> Thanks & Best Regards,
> Songpeng
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, August 17, 2018 3:27 AM
>> To: Li, Songpeng <songpeng.li@intel.com>; edk2-devel@lists.01.org
>> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
>> Subject: Re: [edk2] [PATCH 4/5] NetworkPkg: TlsDxe: Remove the redundant
>> definition.
>>
>> On 08/16/18 03:37, Songpeng Li wrote:
>>> The definition NetLib has been removed.
>>
>> (1) I suggest calling this "lib class dependency", not "definition".
>>
>> (I could have made the same for a GUID removed in a previous patch, but
>> there "definition" seemed acceptable. "GUID dependency" would be an
>> improvement there as well.)
>>
>> The subject line could also say, "remove superfluous dependencies".
>>
>>>
>>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Songpeng Li <songpeng.li@intel.com>
>>> ---
>>>  NetworkPkg/TlsDxe/TlsDxe.inf | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/NetworkPkg/TlsDxe/TlsDxe.inf b/NetworkPkg/TlsDxe/TlsDxe.inf
>>> index 907feb735b..aaea0fc2ff 100644
>>> --- a/NetworkPkg/TlsDxe/TlsDxe.inf
>>> +++ b/NetworkPkg/TlsDxe/TlsDxe.inf
>>> @@ -5,7 +5,7 @@
>>>  #  This module produces EFI TLS (Transport Layer Security) Protocol and EFI
>> TLS
>>>  #  Service Binding Protocol, to provide TLS services.
>>>  #
>>> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>>> +#  Copyright (c) 2016 - 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
>>> @@ -52,7 +52,6 @@
>>>    BaseLib
>>>    UefiLib
>>>    DebugLib
>>> -  NetLib
>>>    BaseCryptLib
>>>    TlsLib
>>>
>>>
>>
>> (2) This patch is incomplete; the file "NetworkPkg/TlsDxe/TlsImpl.h"
>> contains:
>>
>> #include <Library/NetLib.h>
>>
>> That header file is huge, so I won't try to audit, on a source code
>> level, whether TlsDxe makes any NetLib API calls.
>>
>> (Note that even if it does, then the current patch need not necessarily
>> cause a linking failure. Because, a NetLib dependency could be inherited
>> through one of the library instances with which a given platform
>> resolves the *other* lib class dependencies for TlsDxe.)
>>
>> So, please remove the #include too, and if TlsDxe still compiles, please
>> resubmit the patch. (Otherwise, drop the patch, of course.)
>>
>> Thanks!
>> Laszlo

[PATCH 5/5] NetworkPkg: UefiPxeBcDxe: Remove the redundant code.

[Songpeng Li]

The function PxeBcFreeBootFileOption that is never called
have been removed.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Songpeng Li <songpeng.li@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 23 -----------------------
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 10 ----------
 2 files changed, 33 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
index c06b0242bb..5bbc1922a6 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -202,29 +202,6 @@ PxeBcCacheDhcp6Packet (
   return EFI_SUCCESS;
 }
 
-
-/**
-  Free all the nodes in the list for boot file.
-
-  @param[in]  Head            The pointer to the head of list.
-
-**/
-VOID
-PxeBcFreeBootFileOption (
-  IN LIST_ENTRY               *Head
-  )
-{
-  LIST_ENTRY                  *Entry;
-  LIST_ENTRY                  *NextEntry;
-  PXEBC_DHCP6_OPTION_NODE     *Node;
-
-  NET_LIST_FOR_EACH_SAFE (Entry, NextEntry, Head) {
-    Node = NET_LIST_USER_STRUCT (Entry, PXEBC_DHCP6_OPTION_NODE, Link);
-    RemoveEntryList (Entry);
-    FreePool (Node);
-  }
-}
-
 /**
   Retrieve the boot server address using the EFI_DNS6_PROTOCOL.
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
index f3dcd89743..d6e6745f0d 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
@@ -116,16 +116,6 @@ typedef struct {
 } PXEBC_DHCP6_PACKET_CACHE;
 
 
-/**
-  Free all the nodes in the boot file list.
-
-  @param[in]  Head            The pointer to the head of the list.
-
-**/
-VOID
-PxeBcFreeBootFileOption (
-  IN LIST_ENTRY               *Head
-  );
 
 
 /**
-- 
2.18.0.windows.1

Re: [PATCH 5/5] NetworkPkg: UefiPxeBcDxe: Remove the redundant code.

[Laszlo Ersek]

On 08/16/18 03:37, Songpeng Li wrote:
> The function PxeBcFreeBootFileOption that is never called
> have been removed.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1064
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li <songpeng.li@intel.com>
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 23 -----------------------
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 10 ----------
>  2 files changed, 33 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [PATCH 0/5] NetworkPkg: Remove the redundant code and definition.

[Wu, Jiaxin]

Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com> after dropping the patch for TlsDxe.



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Songpeng Li
> Sent: Thursday, August 16, 2018 9:38 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/5] NetworkPkg: Remove the redundant code and
> definition.
> 
> Some redundant functions and definitions which are never calld have been
> removed. These fuctions are manually searched in source files to make
> sure that they are not used elsewhere.
> 
> Songpeng Li (5):
>   NetworkPkg: IScsiDxe: Remove the redundant code and definition.
>   NetworkPkg: IpSecDxe: Remove the redundant code.
>   NetworkPkg: TcpDxe: Remove the redundant code.
>   NetworkPkg: TlsDxe: Remove the redundant definition.
>   NetworkPkg: UefiPxeBcDxe: Remove the redundant code.
> 
>  NetworkPkg/IScsiDxe/IScsiDxe.inf     |  1 -
>  NetworkPkg/IScsiDxe/IScsiProto.c     | 33 ------------
>  NetworkPkg/IpSecDxe/Ikev2/Payload.c  | 18 -------
>  NetworkPkg/IpSecDxe/Ikev2/Utility.c  | 76 ----------------------------
>  NetworkPkg/IpSecDxe/Ikev2/Utility.h  | 67 ------------------------
>  NetworkPkg/TcpDxe/TcpOption.c        | 32 +-----------
>  NetworkPkg/TcpDxe/TcpOption.h        | 18 +------
>  NetworkPkg/TlsDxe/TlsDxe.inf         |  3 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 23 ---------
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 10 ----
>  10 files changed, 3 insertions(+), 278 deletions(-)
> 
> --
> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel