From c0d494b5a7e14f007a425c7c18f24a739d596bce Mon Sep 17 00:00:00 2001 From: tye1 Date: Wed, 31 Aug 2011 12:00:09 +0000 Subject: [PATCH] 1. Update iSCSI UI to be more user-friendly. 2. Fix potential memory leak issue in IScsiConfig.c. Signed-off-by: tye Reviewed-by: xdu2 Reviewed-by: lgao4 git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@12245 6f19259b-4bc3-4df7-8a09-765794883524 --- NetworkPkg/IScsiDxe/IScsiConfig.c | 273 ++++++++++++------- NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h | 6 - NetworkPkg/IScsiDxe/IScsiConfigStrings.uni | Bin 13314 -> 12526 bytes NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr | 66 +---- NetworkPkg/IScsiDxe/IScsiDriver.h | 1 + NetworkPkg/IScsiDxe/IScsiMisc.c | 10 +- 6 files changed, 182 insertions(+), 174 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c b/NetworkPkg/IScsiDxe/IScsiConfig.c index 3c299be949..e6852a9b0a 100644 --- a/NetworkPkg/IScsiDxe/IScsiConfig.c +++ b/NetworkPkg/IScsiDxe/IScsiConfig.c @@ -421,15 +421,16 @@ IScsiConvertAttemptConfigDataToIfrNvData ( AsciiStrToUnicodeStr (Attempt->AttemptName, IfrNvData->AttemptName); } - /** Convert the IFR data to iSCSI configuration data. - @param[in] IfrNvData The IFR nv data. + @param[in] IfrNvData Point to ISCSI_CONFIG_IFR_NVDATA. @param[in, out] Attempt The iSCSI attempt config data. @retval EFI_INVALID_PARAMETER Any input or configured parameter is invalid. @retval EFI_NOT_FOUND Cannot find the corresponding variable. + @retval EFI_OUT_OF_RESOURCES The operation is failed due to lack of resources. + @retval EFI_ABORTED The operation is aborted. @retval EFI_SUCCESS The operation is completed successfully. **/ @@ -451,6 +452,11 @@ IScsiConvertIfrNvDataToAttemptConfigData ( CHAR16 IpMode[64]; ISCSI_NIC_INFO *NicInfo; EFI_INPUT_KEY Key; + UINT8 *AttemptConfigOrder; + UINTN AttemptConfigOrderSize; + UINT8 *AttemptOrderTmp; + UINTN TotalNumber; + EFI_STATUS Status; if (IfrNvData == NULL || Attempt == NULL) { return EFI_INVALID_PARAMETER; @@ -614,6 +620,61 @@ IScsiConvertIfrNvDataToAttemptConfigData ( } } + // + // Update the iSCSI Mode data and record it in attempt help info. + // + Attempt->SessionConfigData.Enabled = IfrNvData->Enabled; + if (IfrNvData->Enabled == ISCSI_DISABLED) { + UnicodeSPrint (IScsiMode, 64, L"Disabled"); + } else if (IfrNvData->Enabled == ISCSI_ENABLED) { + UnicodeSPrint (IScsiMode, 64, L"Enabled"); + } else if (IfrNvData->Enabled == ISCSI_ENABLED_FOR_MPIO) { + UnicodeSPrint (IScsiMode, 64, L"Enabled for MPIO"); + } + + if (IfrNvData->IpMode == IP_MODE_IP4) { + UnicodeSPrint (IpMode, 64, L"IP4"); + } else if (IfrNvData->IpMode == IP_MODE_IP6) { + UnicodeSPrint (IpMode, 64, L"IP6"); + } else if (IfrNvData->IpMode == IP_MODE_AUTOCONFIG) { + UnicodeSPrint (IpMode, 64, L"Autoconfigure"); + } + + NicInfo = IScsiGetNicInfoByIndex (Attempt->NicIndex); + if (NicInfo == NULL) { + return EFI_NOT_FOUND; + } + + MacString = (CHAR16 *) AllocateZeroPool (ISCSI_MAX_MAC_STRING_LEN * sizeof (CHAR16)); + if (MacString == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + AsciiStrToUnicodeStr (Attempt->MacString, MacString); + + UnicodeSPrint ( + mPrivate->PortString, + (UINTN) ISCSI_NAME_IFR_MAX_SIZE, + L"MAC: %s, PFA: Bus %d | Dev %d | Func %d, iSCSI mode: %s, IP version: %s", + MacString, + NicInfo->BusNumber, + NicInfo->DeviceNumber, + NicInfo->FunctionNumber, + IScsiMode, + IpMode + ); + + Attempt->AttemptTitleHelpToken = HiiSetString ( + mCallbackInfo->RegisteredHandle, + Attempt->AttemptTitleHelpToken, + mPrivate->PortString, + NULL + ); + if (Attempt->AttemptTitleHelpToken == 0) { + FreePool (MacString); + return EFI_OUT_OF_RESOURCES; + } + // // Check whether this attempt is an existing one. // @@ -683,7 +744,70 @@ IScsiConvertIfrNvDataToAttemptConfigData ( } } - } else if (ExistAttempt == NULL && IfrNvData->Enabled != ISCSI_DISABLED) { + } else if (ExistAttempt == NULL) { + // + // When a new attempt is created, pointer of the attempt is saved to + // mPrivate->NewAttempt, and also saved to mCallbackInfo->Current in + // IScsiConfigProcessDefault. If input Attempt does not match any existing + // attempt, it should be a new created attempt. Save it to system now. + // + ASSERT (Attempt == mPrivate->NewAttempt); + + // + // Save current order number for this attempt. + // + AttemptConfigOrder = IScsiGetVariableAndSize ( + L"AttemptOrder", + &mVendorGuid, + &AttemptConfigOrderSize + ); + + TotalNumber = AttemptConfigOrderSize / sizeof (UINT8); + TotalNumber++; + + // + // Append the new created attempt order to the end. + // + AttemptOrderTmp = AllocateZeroPool (TotalNumber * sizeof (UINT8)); + if (AttemptOrderTmp == NULL) { + if (AttemptConfigOrder != NULL) { + FreePool (AttemptConfigOrder); + } + return EFI_OUT_OF_RESOURCES; + } + + if (AttemptConfigOrder != NULL) { + CopyMem (AttemptOrderTmp, AttemptConfigOrder, AttemptConfigOrderSize); + FreePool (AttemptConfigOrder); + } + + AttemptOrderTmp[TotalNumber - 1] = Attempt->AttemptConfigIndex; + AttemptConfigOrder = AttemptOrderTmp; + AttemptConfigOrderSize = TotalNumber * sizeof (UINT8); + + Status = gRT->SetVariable ( + L"AttemptOrder", + &mVendorGuid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, + AttemptConfigOrderSize, + AttemptConfigOrder + ); + FreePool (AttemptConfigOrder); + if (EFI_ERROR (Status)) { + return Status; + } + + // + // Insert new created attempt to array. + // + InsertTailList (&mPrivate->AttemptConfigs, &Attempt->Link); + mPrivate->AttemptCount++; + // + // Reset mPrivate->NewAttempt to NULL, which indicates none attempt is created + // but not saved now. + // + mPrivate->NewAttempt = NULL; + if (IfrNvData->Enabled == ISCSI_ENABLED_FOR_MPIO) { // // This new Attempt is enabled for MPIO; enable the multipath mode. @@ -693,61 +817,8 @@ IScsiConvertIfrNvDataToAttemptConfigData ( } else if (IfrNvData->Enabled == ISCSI_ENABLED) { mPrivate->SinglePathCount++; } - } - // - // Update the iSCSI Mode data and record it in attempt help info. - // - Attempt->SessionConfigData.Enabled = IfrNvData->Enabled; - if (IfrNvData->Enabled == ISCSI_DISABLED) { - UnicodeSPrint (IScsiMode, 64, L"Disabled"); - } else if (IfrNvData->Enabled == ISCSI_ENABLED) { - UnicodeSPrint (IScsiMode, 64, L"Enabled"); - } else if (IfrNvData->Enabled == ISCSI_ENABLED_FOR_MPIO) { - UnicodeSPrint (IScsiMode, 64, L"Enabled for MPIO"); - } - - if (IfrNvData->IpMode == IP_MODE_IP4) { - UnicodeSPrint (IpMode, 64, L"IP4"); - } else if (IfrNvData->IpMode == IP_MODE_IP6) { - UnicodeSPrint (IpMode, 64, L"IP6"); - } else if (IfrNvData->IpMode == IP_MODE_AUTOCONFIG) { - UnicodeSPrint (IpMode, 64, L"Autoconfigure"); - } - - NicInfo = IScsiGetNicInfoByIndex (Attempt->NicIndex); - if (NicInfo == NULL) { - return EFI_NOT_FOUND; - } - - MacString = (CHAR16 *) AllocateZeroPool (ISCSI_MAX_MAC_STRING_LEN * sizeof (CHAR16)); - if (MacString == NULL) { - return EFI_OUT_OF_RESOURCES; - } - - AsciiStrToUnicodeStr (Attempt->MacString, MacString); - - UnicodeSPrint ( - mPrivate->PortString, - (UINTN) ISCSI_NAME_IFR_MAX_SIZE, - L"MAC: %s, PFA: Bus %d | Dev %d | Func %d, iSCSI mode: %s, IP version: %s", - MacString, - NicInfo->BusNumber, - NicInfo->DeviceNumber, - NicInfo->FunctionNumber, - IScsiMode, - IpMode - ); - - Attempt->AttemptTitleHelpToken = HiiSetString ( - mCallbackInfo->RegisteredHandle, - Attempt->AttemptTitleHelpToken, - mPrivate->PortString, - NULL - ); - if (Attempt->AttemptTitleHelpToken == 0) { - FreePool (MacString); - return EFI_OUT_OF_RESOURCES; + IScsiConfigUpdateAttempt (); } // @@ -919,7 +990,7 @@ IScsiConfigAddAttempt ( MacString ); - UnicodeSPrint (mPrivate->PortString, (UINTN) ISCSI_NAME_IFR_MAX_SIZE, L"Port %s", MacString); + UnicodeSPrint (mPrivate->PortString, (UINTN) ISCSI_NAME_IFR_MAX_SIZE, L"MAC %s", MacString); PortTitleToken = HiiSetString ( mCallbackInfo->RegisteredHandle, 0, @@ -1081,7 +1152,8 @@ IScsiConfigDeleteAttempts ( AttemptNewOrder = AllocateZeroPool (AttemptConfigOrderSize); if (AttemptNewOrder == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto Error; } Total = AttemptConfigOrderSize / sizeof (UINT8); @@ -1193,8 +1265,13 @@ IScsiConfigDeleteAttempts ( ); Error: - FreePool (AttemptConfigOrder); - FreePool (AttemptNewOrder); + if (AttemptConfigOrder != NULL) { + FreePool (AttemptConfigOrder); + } + + if (AttemptNewOrder != NULL) { + FreePool (AttemptNewOrder); + } return Status; } @@ -1530,10 +1607,19 @@ IScsiConfigProcessDefault ( UINT8 *AttemptConfigOrder; UINTN AttemptConfigOrderSize; UINTN TotalNumber; - UINT8 *AttemptOrderTmp; UINTN Index; - EFI_STATUS Status; + // + // Free any attempt that is previously created but not saved to system. + // + if (mPrivate->NewAttempt != NULL) { + FreePool (mPrivate->NewAttempt); + mPrivate->NewAttempt = NULL; + } + + // + // Is User creating a new attempt? + // NewAttempt = FALSE; if ((KeyValue >= KEY_MAC_ENTRY_BASE) && @@ -1567,7 +1653,7 @@ IScsiConfigProcessDefault ( } // - // Create the new attempt and save to NVR. + // Create new attempt. // AttemptConfigData = AllocateZeroPool (sizeof (ISCSI_ATTEMPT_CONFIG_NVDATA)); @@ -1612,46 +1698,14 @@ IScsiConfigProcessDefault ( TotalNumber++; - // - // Append the new created attempt order to the end. - // - AttemptOrderTmp = AllocateZeroPool (TotalNumber * sizeof (UINT8)); - if (AttemptOrderTmp == NULL) { - FreePool (AttemptConfigData); - if (AttemptConfigOrder != NULL) { - FreePool (AttemptConfigOrder); - } - return EFI_OUT_OF_RESOURCES; - } - - if (AttemptConfigOrder != NULL) { - CopyMem (AttemptOrderTmp, AttemptConfigOrder, AttemptConfigOrderSize); - FreePool (AttemptConfigOrder); - } - - AttemptOrderTmp[TotalNumber - 1] = CurrentAttemptConfigIndex; - AttemptConfigOrder = AttemptOrderTmp; - AttemptConfigOrderSize = TotalNumber * sizeof (UINT8); - - Status = gRT->SetVariable ( - L"AttemptOrder", - &mVendorGuid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, - AttemptConfigOrderSize, - AttemptConfigOrder - ); - FreePool (AttemptConfigOrder); - if (EFI_ERROR (Status)) { - FreePool (AttemptConfigData); - return Status; - } - // // Record the mapping between attempt order and attempt's configdata. // AttemptConfigData->AttemptConfigIndex = CurrentAttemptConfigIndex; - InsertTailList (&mPrivate->AttemptConfigs, &AttemptConfigData->Link); - mPrivate->AttemptCount++; + + if (AttemptConfigOrder != NULL) { + FreePool (AttemptConfigOrder); + } // // Record the MAC info in Config Data. @@ -1708,6 +1762,13 @@ IScsiConfigProcessDefault ( ); UnicodeStrToAsciiStr (mPrivate->PortString, AttemptConfigData->AttemptName); + // + // Save the created Attempt temporarily. If user does not save the attempt + // by press 'KEY_SAVE_ATTEMPT_CONFIG' later, iSCSI driver would know that + // and free resources. + // + mPrivate->NewAttempt = (VOID *) AttemptConfigData; + } else { // // Determine which Attempt user has selected to configure. @@ -1734,10 +1795,11 @@ IScsiConfigProcessDefault ( IScsiConvertAttemptConfigDataToIfrNvData (AttemptConfigData, IfrNvData); + // + // Update current attempt to be a new created attempt or an existing attempt. + // mCallbackInfo->Current = AttemptConfigData; - IScsiConfigUpdateAttempt (); - return EFI_SUCCESS; } @@ -2496,6 +2558,13 @@ IScsiConfigFormUnload ( ASSERT (mPrivate->NicCount == 0); + // + // Free attempt is created but not saved to system. + // + if (mPrivate->NewAttempt != NULL) { + FreePool (mPrivate->NewAttempt); + } + FreePool (mPrivate); mPrivate = NULL; diff --git a/NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h b/NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h index 211b4ab034..ee66e2aae0 100644 --- a/NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h +++ b/NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h @@ -102,12 +102,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define KEY_IGNORE_DELETE_ATTEMPT 0x115 #define KEY_DELETE_ATTEMPT 0x116 -#define KEY_KERBEROS_USER_NAME 0x117 -#define KEY_KERBEROS_USER_SECRET 0x118 -#define KEY_KERBEROS_KDC_NAME 0x119 -#define KEY_KERBEROS_KDC_REALM 0x11a -#define KEY_KERBEROS_KDC_IP_ADDR 0x11b - #define KEY_IP_MODE 0x11c #define KEY_AUTH_TYPE 0x11d #define KEY_CONFIG_ISID 0x11e diff --git a/NetworkPkg/IScsiDxe/IScsiConfigStrings.uni b/NetworkPkg/IScsiDxe/IScsiConfigStrings.uni index 8df9cef13e70e7fcb0bcab112e65be6b284f8401..57ef9c8d776e28ed9a602a47e4852a6b4b9d77ef 100644 GIT binary patch delta 494 zcmZ8eJxjw-6g`4UTd_!$AW}=zRoX?|#i>I@t$*N|rZ#Aj(#A+9rGtOqZca{44nn}i zrT!EB1N{er=f0?@g!|t8c=z0MZpOW-dAcpiW1%MZ#qwMeM>vp|T01M0rVEP^0wgrU zGnS9)dTR(5LyTw%e1wb){UP6+A!4S(OhC^?6NlL49U^AdN8RSMvWH8>gD0=^%h%ua zZB_!bu}^E#?_j_BFn(D+22SBdj{m^+e%6#!#?Gg zV&|ByiZO&ZM^2*4T$dRgnM+F0qo?HmkI!LL=nsm#*S2KhRI(H2xst8aof&yps>)lV zB$Iks#;dN}tZfD@u0eZTlJjv&0kMt#t~cbwh%b$mKeEhXyAqF_w5G!Jx9CdsG}PhK Q>$5{u`Q)>a^%{4xpTj_DX8-^I delta 674 zcmaJQv4kG#kZ`Y>!2f^&3 zMJ?O4?N9g{I?t~~rHl9GzW44q_ndpDYu#2~?x*~1R8fHj8+EQWm&N@GE%Ci(JF429 z7GGZXY>X)xS@5Addr[Index] >> 4) & 0x0F]; Str[3 * Index + 1] = (CHAR16) IScsiHexString[Mac->Addr[Index] & 0x0F]; - Str[3 * Index + 2] = L'-'; + Str[3 * Index + 2] = L':'; } String = &Str[3 * Index - 1] ; @@ -640,6 +640,14 @@ IScsiRemoveNic ( } } + // + // Free attempt is created but not saved to system. + // + if (mPrivate->NewAttempt != NULL) { + FreePool (mPrivate->NewAttempt); + mPrivate->NewAttempt = NULL; + } + return EFI_SUCCESS; }