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

6 files changed, 181 insertions, 173 deletions

diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c b/NetworkPkg/IScsiDxe/IScsiConfig.c
index 3c299be94..e6852a9b0 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;

@@ -615,6 +621,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.

   //

   ExistAttempt = IScsiConfigGetAttemptByConfigIndex (Attempt->AttemptConfigIndex);

@@ -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));

@@ -1613,47 +1699,15 @@ IScsiConfigProcessDefault (
     TotalNumber++;

 

     //

-    // Append the new created attempt order to the end.

+    // Record the mapping between attempt order and attempt's configdata.

     //

-    AttemptOrderTmp = AllocateZeroPool (TotalNumber * sizeof (UINT8));

-    if (AttemptOrderTmp == NULL) {

-      FreePool (AttemptConfigData);

-      if (AttemptConfigOrder != NULL) {

-        FreePool (AttemptConfigOrder);

-      }

-      return EFI_OUT_OF_RESOURCES;

-    }

+    AttemptConfigData->AttemptConfigIndex  = CurrentAttemptConfigIndex;

 

     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;

+      FreePool (AttemptConfigOrder);

     }

 

     //

-    // Record the mapping between attempt order and attempt's configdata.

-    //

-    AttemptConfigData->AttemptConfigIndex  = CurrentAttemptConfigIndex;

-    InsertTailList (&mPrivate->AttemptConfigs, &AttemptConfigData->Link);

-    mPrivate->AttemptCount++;

-

-    //

     // Record the MAC info in Config Data.

     //

     IScsiMacAddrToStr (

@@ -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 211b4ab03..ee66e2aae 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 8df9cef13..57ef9c8d7 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfigStrings.uni
+++ b/NetworkPkg/IScsiDxe/IScsiConfigStrings.uni
diff --git a/NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr b/NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr
index 94b00ee60..0cf4ffeed 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr
+++ b/NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr
@@ -346,75 +346,11 @@ formset
 

     endif;

 

-    suppressif NOT ideqval ISCSI_CONFIG_IFR_NVDATA.AuthenticationType == ISCSI_AUTH_TYPE_KRB;

-

-    checkbox varid = ISCSI_CONFIG_IFR_NVDATA.MutualRequired,

-            prompt = STRING_TOKEN(STR_ISCSI_MUTUAL_REQUIRED),

-            help   = STRING_TOKEN(STR_ISCSI_MUTUAL_REQUIRED_HELP),

-            flags  = 0,

-    endcheckbox;

-    

-    string  varid   = ISCSI_CONFIG_IFR_NVDATA.KerberosUserName,

-            prompt  = STRING_TOKEN(STR_ISCSI_KERBEROS_USER_NAME),

-            help    = STRING_TOKEN(STR_ISCSI_KERBEROS_USER_NAME),

-            flags   = INTERACTIVE,

-            key     = KEY_KERBEROS_USER_NAME,

-            minsize = 0,

-            maxsize = KERBEROS_NAME_MAX_LEN,

-    endstring;

-

-    string  varid    = ISCSI_CONFIG_IFR_NVDATA.KerberosUserSecret,

-            prompt   = STRING_TOKEN(STR_ISCSI_KERBEROS_USER_SECRET),

-            help     = STRING_TOKEN(STR_ISCSI_KERBEROS_USER_SECRET),

-            flags    = INTERACTIVE,

-            key      = KEY_KERBEROS_USER_SECRET,

-            minsize  = KERBEROS_SECRET_MIN_LEN,

-            maxsize  = KERBEROS_SECRET_MAX_LEN,

-    endstring;

-    

-    string  varid   = ISCSI_CONFIG_IFR_NVDATA.KerberosKDCName,

-            prompt  = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_NAME),

-            help    = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_NAME),

-            flags   = INTERACTIVE,

-            key     = KEY_KERBEROS_KDC_NAME,

-            minsize = 0,

-            maxsize = KERBEROS_NAME_MAX_LEN,

-    endstring;

-

-    string  varid    = ISCSI_CONFIG_IFR_NVDATA.KerberosKDCRealm,

-            prompt   = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_REALM),

-            help     = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_REALM),

-            flags    = INTERACTIVE,

-            key      = KEY_KERBEROS_KDC_REALM,

-            minsize  = 0,

-            maxsize  = KERBEROS_NAME_MAX_LEN,

-    endstring;

-

-    string  varid   = ISCSI_CONFIG_IFR_NVDATA.KerberosKDCIp,

-            prompt  = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_IP),

-            help    = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_IP),

-            flags   = INTERACTIVE,

-            key     = KEY_KERBEROS_KDC_IP_ADDR,

-            minsize = IP_MIN_SIZE,

-            maxsize = IP_MAX_SIZE,

-    endstring;

-

-    numeric varid   = ISCSI_CONFIG_IFR_NVDATA.KerberosKDCPort,

-            prompt  = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_PORT),

-            help    = STRING_TOKEN(STR_ISCSI_KERBEROS_KDC_PORT),

-            flags   = 0,

-            minimum = KERBEROS_KDC_PORT_MIN_NUM,

-            maximum = KERBEROS_KDC_PORT_MAX_NUM,

-            step    = 0,

-    endnumeric;

-    

-    endif;

-

     subtitle text = STRING_TOKEN(STR_NULL);

 

     goto FORMID_ATTEMPT_FORM,

     prompt = STRING_TOKEN (STR_SAVE_CHANGES),

-    help   = STRING_TOKEN (STR_SAVE_CHANGES),

+    help   = STRING_TOKEN (STR_SAVE_CHANGES_HELP),

     flags  = INTERACTIVE,

     key    = KEY_SAVE_ATTEMPT_CONFIG;

 

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.h b/NetworkPkg/IScsiDxe/IScsiDriver.h
index ad6b83a3c..04866f0ed 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.h
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.h
@@ -55,6 +55,7 @@ typedef struct {
   LIST_ENTRY      AttemptConfigs;       // User configured Attempt list.

   CHAR8           InitiatorName[ISCSI_NAME_MAX_SIZE];

   UINTN           InitiatorNameLength;

+  VOID            *NewAttempt;          // Attempt is created but not saved.

 } ISCSI_PRIVATE_DATA;

 

 extern ISCSI_PRIVATE_DATA                 *mPrivate;

diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index a697659f4..2cf3db01b 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -297,7 +297,7 @@ IScsiMacAddrToStr (
   for (Index = 0; Index < Len; Index++) {

     Str[3 * Index]      = (CHAR16) IScsiHexString[(Mac->Addr[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;

 }