In my opinion, when AttemptConfigData->AttemptTitleHelpToken equals to 0, the
system is running out of resources. So we 'd better to return the error now
instead of continue processing.
Best Regards,
Ye Ting
-----Original Message-----
From: Zhang, Lubo
Sent: Monday, June 20, 2016 5:57 PM
To: Ye, Ting <ting...@intel.com>; edk2-devel@lists.01.org
Cc: Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: RE: [PATCH v2] NetworkPkg: Replace ASSERT with error handling in Http
boot and IScsi
Do you mean that if we already find an attempt from the Variable and configure
some parameters, but finally get string Id AttemptTitleHelpToken failed. If we
continue, this attempt will not be added to global link list which will be used
frequently and lead to a wrong code logic later, so we should return error
status.
Thanks
lubo
-----Original Message-----
From: Ye, Ting
Sent: Monday, June 20, 2016 5:01 PM
To: Zhang, Lubo <lubo.zh...@intel.com>; edk2-devel@lists.01.org
Cc: Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: RE: [PATCH v2] NetworkPkg: Replace ASSERT with error handling in Http
boot and IScsi
Hi Lubo,
I recommend to return EFI_OUT_OF_RESOURCES instead of continue to process the
attempts in below codes. What do you think?
EFI_STATUS
IScsiGetConfigData (
IN ISCSI_DRIVER_DATA *Private
@@ -1290,11 +1291,13 @@ IScsiGetConfigData (
mCallbackInfo->RegisteredHandle,
0,
mPrivate->PortString,
NULL
);
- ASSERT (AttemptConfigData->AttemptTitleHelpToken != 0);
+ if (AttemptConfigData->AttemptTitleHelpToken == 0) {
+ continue;
+ }
Best Regards,
Ye Ting
-----Original Message-----
From: Zhang, Lubo
Sent: Monday, June 20, 2016 1:57 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin
<jiaxin...@intel.com>
Subject: [PATCH v2] NetworkPkg: Replace ASSERT with error handling in Http boot
and IScsi
v2:
*Fix some memory leak issue.
This patch is used to replace ASSERT with error handling in Http boot Driver
and IScsi driver.
Cc: Ye Ting <ting...@intel.com>
Cc: Fu Siyuan <siyuan...@intel.com>
Cc: Wu Jiaxin <jiaxin...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zh...@intel.com>
---
NetworkPkg/HttpBootDxe/HttpBootConfig.c | 5 +++--
NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 5 ++++-
NetworkPkg/IScsiDxe/IScsiConfig.c | 8 ++++++--
NetworkPkg/IScsiDxe/IScsiDriver.c | 1 +
NetworkPkg/IScsiDxe/IScsiMisc.c | 5 ++++-
NetworkPkg/IScsiDxe/IScsiProto.c | 17 +++++++++++++----
6 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 04c2f3e..3708995 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -271,11 +271,13 @@ HttpBootFormExtractConfig (
// followed by "&OFFSET=0&WIDTH=WWWWWWWWWWWWWWWW" followed by a
Null-terminator
//
ConfigRequestHdr = HiiConstructConfigHdr (&gHttpBootConfigGuid,
mHttpBootConfigStorageName, CallbackInfo->ChildHandle);
Size = (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16);
ConfigRequest = AllocateZeroPool (Size);
- ASSERT (ConfigRequest != NULL);
+ if (ConfigRequest == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
AllocatedRequest = TRUE;
UnicodeSPrint (ConfigRequest, Size, L"%s&OFFSET=0&WIDTH=%016LX",
ConfigRequestHdr, (UINT64)BufferSize);
FreePool (ConfigRequestHdr);
}
@@ -462,11 +464,10 @@ HttpBootFormCallback (
case KEY_INITIATOR_URI:
//
// Get user input URI string
//
Uri = HiiGetString (CallbackInfo->RegisteredHandle, Value->string, NULL);
- ASSERT (Uri != NULL);
if (Uri == NULL) {
return EFI_UNSUPPORTED;
}
//
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
index 0157095..9ea421d 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
@@ -399,10 +399,11 @@ HttpBootCacheDhcp6Offer (
@retval EFI_SUCCESS Told the EFI DHCPv6 Protocol driver to
continue the DHCP process.
@retval EFI_NOT_READY Only used in the Dhcp6Selecting state. The EFI
DHCPv6 Protocol
driver will continue to wait for more packets.
@retval EFI_ABORTED Told the EFI DHCPv6 Protocol driver to abort
the current process.
+ @retval EFI_OUT_OF_RESOURCES There are not enough resources.
**/
EFI_STATUS
EFIAPI
HttpBootDhcp6CallBack (
@@ -449,11 +450,13 @@ HttpBootDhcp6CallBack (
Status = EFI_ABORTED;
} else {
ASSERT (NewPacket != NULL);
SelectAd = &Private->OfferBuffer[Private->SelectIndex -
1].Dhcp6.Packet.Offer;
*NewPacket = AllocateZeroPool (SelectAd->Size);
- ASSERT (*NewPacket != NULL);
+ if (*NewPacket == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
CopyMem (*NewPacket, SelectAd, SelectAd->Size);
}
break;
default:
diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c
b/NetworkPkg/IScsiDxe/IScsiConfig.c
index 69a4003..8f06a07 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -1,9 +1,9 @@
/** @file
Helper functions for configuring or getting the parameters relating to iSCSI.
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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
@@ -1957,11 +1957,15 @@ IScsiFormExtractConfig (
// followed by "&OFFSET=0&WIDTH=WWWWWWWWWWWWWWWW" followed by a
Null-terminator
//
ConfigRequestHdr = HiiConstructConfigHdr (&gIScsiConfigGuid,
mVendorStorageName, Private->DriverHandle);
Size = (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16);
ConfigRequest = AllocateZeroPool (Size);
- ASSERT (ConfigRequest != NULL);
+ if (ConfigRequest == NULL) {
+ FreePool (IfrNvData);
+ FreePool (InitiatorName);
+ return EFI_OUT_OF_RESOURCES;
+ }
AllocatedRequest = TRUE;
UnicodeSPrint (ConfigRequest, Size, L"%s&OFFSET=0&WIDTH=%016LX",
ConfigRequestHdr, (UINT64)BufferSize);
FreePool (ConfigRequestHdr);
}
diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 5a121ce..285f151 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -321,10 +321,11 @@ IScsiSupported (
@retval EFI_SUCCES This driver was started.
@retval EFI_ALREADY_STARTED This driver is already running on this device.
@retval EFI_INVALID_PARAMETER Any input parameter is invalid.
@retval EFI_NOT_FOUND There is no sufficient information to establish
the iScsi session.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_DEVICE_ERROR Failed to get TCP connection device path.
@retval EFI_ACCESS_DENIED The protocol could not be removed from the
Handle
because its interfaces are being used.
**/
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index a1f2672..f19f36c 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -1004,10 +1004,11 @@ IScsiDhcpIsConfigured (
@param[in] Private The iSCSI driver data.
@retval EFI_SUCCESS The configuration data is retrieved.
@retval EFI_NOT_FOUND This iSCSI driver is not configured yet.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
**/
EFI_STATUS
IScsiGetConfigData (
IN ISCSI_DRIVER_DATA *Private
@@ -1290,11 +1291,13 @@ IScsiGetConfigData (
mCallbackInfo->RegisteredHandle,
0,
mPrivate->PortString,
NULL
);
- ASSERT (AttemptConfigData->AttemptTitleHelpToken != 0);
+ if (AttemptConfigData->AttemptTitleHelpToken == 0) {
+ continue;
+ }
//
// Record the attempt in global link list.
//
InsertTailList (&mPrivate->AttemptConfigs, &AttemptConfigData->Link); diff
--git a/NetworkPkg/IScsiDxe/IScsiProto.c b/NetworkPkg/IScsiDxe/IScsiProto.c
index 4c4e3c2..c82290e 100644
--- a/NetworkPkg/IScsiDxe/IScsiProto.c
+++ b/NetworkPkg/IScsiDxe/IScsiProto.c
@@ -1,9 +1,9 @@
/** @file
The implementation of iSCSI protocol based on RFC3720.
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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
@@ -729,11 +729,14 @@ IScsiPrepareLoginReq (
if (Nbuf == NULL) {
return NULL;
}
LoginReq = (ISCSI_LOGIN_REQUEST *) NetbufAllocSpace (Nbuf, sizeof
(ISCSI_LOGIN_REQUEST), NET_BUF_TAIL);
- ASSERT (LoginReq != NULL);
+ if (LoginReq == NULL) {
+ NetbufFree (Nbuf);
+ return NULL;
+ }
ZeroMem (LoginReq, sizeof (ISCSI_LOGIN_REQUEST));
//
// Init the login request pdu
//
@@ -1243,11 +1246,14 @@ IScsiReceivePdu (
Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
Header = NetbufAllocSpace (PduHdr, Len, NET_BUF_TAIL);
- ASSERT (Header != NULL);
+ if (Header == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
InsertTailList (NbufList, &PduHdr->List);
//
// First step, receive the BHS of the PDU.
//
@@ -2314,11 +2320,14 @@ IScsiNewDataOutPdu (
// Insert the BHS into the buffer list.
//
InsertTailList (NbufList, &PduHdr->List);
DataOutHdr = (ISCSI_SCSI_DATA_OUT *) NetbufAllocSpace (PduHdr, sizeof
(ISCSI_SCSI_DATA_OUT), NET_BUF_TAIL);
- ASSERT (DataOutHdr != NULL);
+ if (DataOutHdr == NULL) {
+ IScsiFreeNbufList (NbufList);
+ return NULL;
+ }
XferContext = &Tcb->XferContext;
ZeroMem (DataOutHdr, sizeof (ISCSI_SCSI_DATA_OUT));
//
--
1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
