Thanks Lubo. Please address this if you agree. Reviewed-by: Sriram Subramanian <srira...@hpe.com>
Thanks, Sriram. -----Original Message----- From: Zhang, Lubo [mailto:lubo.zh...@intel.com] Sent: Wednesday, April 6, 2016 7:37 AM To: Subramanian, Sriram (EG Servers Platform SW) <srira...@hpe.com>; edk2-devel@lists.01.org Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com> Subject: RE: [patch 2/2] MdeModulePkg: Add new macros and refine codes I think the code seems same, may be "if (Length > IP6_PREFIX_MAX)" more readable. -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Subramanian, Sriram (EG Servers Platform SW) Sent: Tuesday, April 05, 2016 10:21 PM To: Zhang, Lubo <lubo.zh...@intel.com>; edk2-devel@lists.01.org Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com> Subject: Re: [edk2] [patch 2/2] MdeModulePkg: Add new macros and refine codes > @@ -3113,11 +3113,11 @@ NetLibStrToIp6andPrefix ( > Status = EFI_INVALID_PARAMETER; > Length = 0; > while (*PrefixStr != '\0') { > if (NET_IS_DIGIT (*PrefixStr)) { > Length = (UINT8) (Length * 10 + (*PrefixStr - '0')); > - if (Length >= IP6_PREFIX_NUM) { > + if (Length >= (IP6_PREFIX_MAX + 1)) { I think this could be just "if (Length > IP6_PREFIX_MAX)". What do you say? Thanks, Sriram. -----Original Message----- From: Zhang Lubo [mailto:lubo.zh...@intel.com] Sent: Tuesday, April 5, 2016 2:19 PM To: edk2-devel@lists.01.org Cc: Subramanian, Sriram (EG Servers Platform SW) <srira...@hpe.com>; Fu Siyuan <siyuan...@intel.com>; Ye Ting <ting...@intel.com>; Wu Jiaxin <jiaxin...@intel.com> Subject: [patch 2/2] MdeModulePkg: Add new macros and refine codes Add 2 macros inNetLib.h #define IP4_MASK_MAX 32 #define IP6_PREFIX_MAX 128 we will use these two macros to check the max mask/prefix length, instead of #define IP4_MASK_NUM 33 #define IP6_PREFIX_NUM 129 which means a valid number. This will make the code readability and maintainability. Cc: Subramanian Sriram <srira...@hpe.com> Cc: Fu Siyuan <siyuan...@intel.com> Cc: Ye Ting <ting...@intel.com> Cc: Wu Jiaxin <jiaxin...@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Zhang Lubo <lubo.zh...@intel.com> --- MdeModulePkg/Include/Library/NetLib.h | 5 ++++- MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 6 +++--- MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 2 +- MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 2 +- MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c | 2 +- MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c | 8 ++++---- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/MdeModulePkg/Include/Library/NetLib.h b/MdeModulePkg/Include/Library/NetLib.h index b871a85..87f393e 100644 --- a/MdeModulePkg/Include/Library/NetLib.h +++ b/MdeModulePkg/Include/Library/NetLib.h @@ -50,10 +50,13 @@ typedef UINT16 TCP_PORTNO; #define IP4_ADDR_CLASSE 5 #define IP4_MASK_NUM 33 #define IP6_PREFIX_NUM 129 +#define IP4_MASK_MAX 32 +#define IP6_PREFIX_MAX 128 + #define IP6_HOP_BY_HOP 0 #define IP6_DESTINATION 60 #define IP6_ROUTING 43 #define IP6_FRAGMENT 44 #define IP6_AH 51 @@ -228,11 +231,11 @@ typedef struct { // Test the IP's attribute, All the IPs are in host byte order. // #define IP4_IS_MULTICAST(Ip) (((Ip) & 0xF0000000) == 0xE0000000) #define IP4_IS_LOCAL_BROADCAST(Ip) ((Ip) == 0xFFFFFFFF) #define IP4_NET_EQUAL(Ip1, Ip2, NetMask) (((Ip1) & (NetMask)) == ((Ip2) & (NetMask))) -#define IP4_IS_VALID_NETMASK(Ip) (NetGetMaskLength (Ip) != IP4_MASK_NUM) +#define IP4_IS_VALID_NETMASK(Ip) (NetGetMaskLength (Ip) != (IP4_MASK_MAX + 1)) #define IP6_IS_MULTICAST(Ip6) (((Ip6)->Addr[0]) == 0xFF) // // Convert the EFI_IP4_ADDRESS to plain UINT32 IP4 address. diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c index ebc3e12..c7793e2 100644 --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c @@ -563,11 +563,11 @@ NetGetMaskLength ( IN IP4_ADDR NetMask ) { INTN Index; - for (Index = 0; Index < IP4_MASK_NUM; Index++) { + for (Index = 0; Index <= IP4_MASK_MAX; Index++) { if (NetMask == gIp4AllMasks[Index]) { break; } } @@ -792,11 +792,11 @@ NetIp6IsNetEqual ( { UINT8 Byte; UINT8 Bit; UINT8 Mask; - ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength < IP6_PREFIX_NUM)); + ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <= + IP6_PREFIX_MAX)); if (PrefixLength == 0) { return TRUE; } @@ -3113,11 +3113,11 @@ NetLibStrToIp6andPrefix ( Status = EFI_INVALID_PARAMETER; Length = 0; while (*PrefixStr != '\0') { if (NET_IS_DIGIT (*PrefixStr)) { Length = (UINT8) (Length * 10 + (*PrefixStr - '0')); - if (Length >= IP6_PREFIX_NUM) { + if (Length >= (IP6_PREFIX_MAX + 1)) { goto Exit; } } else { goto Exit; } diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c index 17e0247..d0fa132 100644 --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c @@ -414,11 +414,11 @@ Ip4Config2BuildDefaultRouteTable ( // is, interlevel the route entry from the instance's route area // and those from the default route table's route area. // Count = 0; - for (Index = IP4_MASK_NUM - 1; Index >= 0; Index--) { + for (Index = IP4_MASK_MAX; Index >= 0; Index--) { NET_LIST_FOR_EACH (Entry, &(IpSb->DefaultRouteTable->RouteArea[Index])) { RtEntry = NET_LIST_USER_STRUCT (Entry, IP4_ROUTE_ENTRY, Link); EFI_IP4 (Table[Count].SubnetAddress) = HTONL (RtEntry->Dest & RtEntry->Netmask); diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c index 669e041..0231725 100644 --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c @@ -579,11 +579,11 @@ Ip4SetAddress ( Interface->SubnetBrdcast = (IpAddr | ~SubnetMask); Type = NetGetIpClass (IpAddr); ASSERT (Type <= IP4_ADDR_CLASSC); Len = NetGetMaskLength (SubnetMask); - ASSERT (Len < IP4_MASK_NUM); + ASSERT (Len <= IP4_MASK_MAX); Netmask = gIp4AllMasks[MIN (Len, Type << 3)]; Interface->NetBrdcast = (IpAddr | ~Netmask); // // Do clean up for Arp child diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c index 58adba7..e733816 100644 --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c @@ -836,11 +836,11 @@ Ip4StationAddressValid ( } // // Only support the continuous net masks // - if ((Len = NetGetMaskLength (Netmask)) == IP4_MASK_NUM) { + if ((Len = NetGetMaskLength (Netmask)) == (IP4_MASK_MAX + 1)) { return FALSE; } // // Station address can't be class D or class E address diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c index ea0023d..f0dea75 100644 --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c @@ -202,11 +202,11 @@ Ip4CreateRouteTable ( } RtTable->RefCnt = 1; RtTable->TotalNum = 0; - for (Index = 0; Index < IP4_MASK_NUM; Index++) { + for (Index = 0; Index <= IP4_MASK_MAX; Index++) { InitializeListHead (&(RtTable->RouteArea[Index])); } RtTable->Next = NULL; @@ -239,11 +239,11 @@ Ip4FreeRouteTable ( } // // Free all the route table entry and its route cache. // - for (Index = 0; Index < IP4_MASK_NUM; Index++) { + for (Index = 0; Index <= IP4_MASK_MAX; Index++) { NET_LIST_FOR_EACH_SAFE (Entry, Next, &(RtTable->RouteArea[Index])) { RtEntry = NET_LIST_USER_STRUCT (Entry, IP4_ROUTE_ENTRY, Link); RemoveEntryList (Entry); Ip4FreeRouteEntry (RtEntry); @@ -467,11 +467,11 @@ Ip4FindRouteEntry ( IP4_ROUTE_TABLE *Table; INTN Index; RtEntry = NULL; - for (Index = IP4_MASK_NUM - 1; Index >= 0; Index--) { + for (Index = IP4_MASK_MAX; Index >= 0; Index--) { for (Table = RtTable; Table != NULL; Table = Table->Next) { NET_LIST_FOR_EACH (Entry, &Table->RouteArea[Index]) { RtEntry = NET_LIST_USER_STRUCT (Entry, IP4_ROUTE_ENTRY, Link); if (IP4_NET_EQUAL (RtEntry->Dest, Dst, RtEntry->Netmask)) { @@ -639,11 +639,11 @@ Ip4BuildEfiRouteTable ( // is, interlevel the route entry from the instance's route area // and those from the default route table's route area. // Count = 0; - for (Index = IP4_MASK_NUM - 1; Index >= 0; Index--) { + for (Index = IP4_MASK_MAX; Index >= 0; Index--) { for (RtTable = IpInstance->RouteTable; RtTable != NULL; RtTable = RtTable->Next) { NET_LIST_FOR_EACH (Entry, &(RtTable->RouteArea[Index])) { RtEntry = NET_LIST_USER_STRUCT (Entry, IP4_ROUTE_ENTRY, Link); EFI_IP4 (Table[Count].SubnetAddress) = HTONL (RtEntry->Dest & RtEntry->Netmask); -- 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
