Re: [edk2] [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.
Reviewed-by: Jiaxin Wu> -Original Message- > From: Wang, Fan > Sent: Wednesday, January 3, 2018 10:32 AM > To: edk2-devel@lists.01.org > Cc: Wang, Fan ; Fu, Siyuan ; Ye, > Ting ; Wu, Jiaxin > Subject: [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and > ASSERT handling. > > From: Wang Fan > > * 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 > Cc: Ye Ting > Cc: Jiaxin Wu > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wang Fan > --- > 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. > +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved. > (C) Copyright 2015 Hewlett Packard Enterprise Development LP > 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] BufLenThe 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 UINT32Level, > @@ -322,10 +325,11 @@ SyslogBuildPacket ( >IN UINT8 *Message, >IN UINT32BufLen, >OUT CHAR8 *Buf >) > { > + EFI_STATUSStatus; >ETHER_HEAD*Ether; >IP4_HEAD *Ip4; >EFI_UDP_HEADER*Udp4; >EFI_TIME Time; >UINT32Pri; > @@ -377,12 +381,14 @@ SyslogBuildPacket ( > >// >// Build the syslog message body with Timestamp machine module > Message >// >Pri = ((NET_SYSLOG_FACILITY & 31) << 3) | (Level & 7); > - gRT->GetTime (, NULL); > - ASSERT ((Time.Month <= 12) && (Time.Month >= 1)); > + Status = gRT->GetTime (, 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. > >@returnThe 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
[edk2] [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.
From: Wang Fan* 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 Cc: Ye Ting Cc: Jiaxin Wu Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wang Fan --- 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. +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2015 Hewlett Packard Enterprise Development LP 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] BufLenThe 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 UINT32Level, @@ -322,10 +325,11 @@ SyslogBuildPacket ( IN UINT8 *Message, IN UINT32BufLen, OUT CHAR8 *Buf ) { + EFI_STATUSStatus; ETHER_HEAD*Ether; IP4_HEAD *Ip4; EFI_UDP_HEADER*Udp4; EFI_TIME Time; UINT32Pri; @@ -377,12 +381,14 @@ SyslogBuildPacket ( // // Build the syslog message body with Timestamp machine module Message // Pri = ((NET_SYSLOG_FACILITY & 31) << 3) | (Level & 7); - gRT->GetTime (, NULL); - ASSERT ((Time.Month <= 12) && (Time.Month >= 1)); + Status = gRT->GetTime (, 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. @returnThe 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