OpenSubKeyPath instead of Path parameter of OpenSubKey (better interface + should ease implementation of ACL)

+ Additional key open failure tests

Source commit: eb41469ccb54bd2b532ca64f64b399a3944b2b1c

source/core/Configuration.cpp

@@ -211,7 +211,7 @@ UnicodeString __fastcall TConfiguration::PropertyToKey(const UnicodeString & Pro
 }
 //---------------------------------------------------------------------------
 #define BLOCK(KEY, CANCREATE, BLOCK) \
-  if (Storage->OpenSubKey(KEY, CANCREATE, true)) try { BLOCK } __finally { Storage->CloseSubKey(); }
+  if (Storage->OpenSubKeyPath(KEY, CANCREATE)) try { BLOCK } __finally { Storage->CloseSubKeyPath(); }
 #define KEY(TYPE, VAR) KEYEX(TYPE, VAR, PropertyToKey(TEXT(#VAR)))
 #define REGCONFIG(CANCREATE) \
   BLOCK(L"Interface", CANCREATE, \

source/core/HierarchicalStorage.cpp

@@ -105,7 +105,6 @@ UnicodeString __fastcall UnMungeIniName(const UnicodeString & Str)
 __fastcall THierarchicalStorage::THierarchicalStorage(const UnicodeString & AStorage)
 {
   FStorage = AStorage;
-  FKeyHistory = new TStringList();
   AccessMode = smRead;
   Explicit = false;
   ForceSave = false;
@@ -118,7 +117,6 @@ __fastcall THierarchicalStorage::THierarchicalStorage(const UnicodeString & ASto
 //---------------------------------------------------------------------------
 __fastcall THierarchicalStorage::~THierarchicalStorage()
 {
-  delete FKeyHistory;
 }
 //---------------------------------------------------------------------------
 void __fastcall THierarchicalStorage::Flush()
@@ -132,9 +130,9 @@ void __fastcall THierarchicalStorage::SetAccessMode(TStorageAccessMode value)
 //---------------------------------------------------------------------------
 UnicodeString __fastcall THierarchicalStorage::GetCurrentSubKeyMunged()
 {
-  if (FKeyHistory->Count > 0)
+  if (!FKeyHistory.empty())
   {
-    return FKeyHistory->Strings[FKeyHistory->Count - 1];
+    return FKeyHistory.back().Key;
   }
   else
   {
@@ -163,47 +161,81 @@ UnicodeString __fastcall THierarchicalStorage::MungeKeyName(const UnicodeString
   return Result;
 }
 //---------------------------------------------------------------------------
-bool __fastcall THierarchicalStorage::OpenSubKey(const UnicodeString & Key, bool CanCreate, bool Path)
+bool __fastcall THierarchicalStorage::OpenSubKeyPath(const UnicodeString & KeyPath, bool CanCreate)
 {
-  UnicodeString MungedKey;
-  if (Path)
+  DebugAssert(!KeyPath.IsEmpty() && (KeyPath[KeyPath.Length()] != L'\\'));
+  bool Result;
+  UnicodeString Buf(KeyPath);
+  int Opens = 0;
+  while (!Buf.IsEmpty())
   {
-    DebugAssert(Key.IsEmpty() || (Key[Key.Length()] != L'\\'));
-    UnicodeString Buf(Key);
-    while (!Buf.IsEmpty())
+    UnicodeString SubKey = CutToChar(Buf, L'\\', false);
+    Result = OpenSubKey(SubKey, CanCreate);
+    if (Result)
     {
-      if (!MungedKey.IsEmpty())
-      {
-        MungedKey += L'\\';
-      }
-      MungedKey += MungeKeyName(CutToChar(Buf, L'\\', false));
+      Opens++;
     }
   }
+
+  if (Result)
+  {
+    FKeyHistory.back().Levels = Opens;
+  }
   else
   {
-    MungedKey = MungeKeyName(Key);
+    while (Opens > 0)
+    {
+      CloseSubKey();
+      Opens--;
+    }
   }
 
+  return Result;
+}
+//---------------------------------------------------------------------------
+bool __fastcall THierarchicalStorage::OpenSubKey(const UnicodeString & Key, bool CanCreate)
+{
+  UnicodeString MungedKey = MungeKeyName(Key);
+
   bool Result = DoOpenSubKey(MungedKey, CanCreate);
 
   if (Result)
   {
-    FKeyHistory->Add(IncludeTrailingBackslash(CurrentSubKey+MungedKey));
+    TKeyEntry Entry;
+    Entry.Key = IncludeTrailingBackslash(CurrentSubKey + MungedKey);
+    Entry.Levels = 1;
+    FKeyHistory.push_back(Entry);
   }
 
   return Result;
 }
 //---------------------------------------------------------------------------
-void __fastcall THierarchicalStorage::CloseSubKey()
+void __fastcall THierarchicalStorage::CloseSubKeyPath()
 {
-  if (FKeyHistory->Count == 0)
+  if (FKeyHistory.empty())
   {
     throw Exception(UnicodeString());
   }
-  else
+
+  int Levels = FKeyHistory.back().Levels;
+  FKeyHistory.back().Levels = 1; // to satify the assertion in CloseSubKey()
+  while (Levels > 0)
+  {
+    CloseSubKey();
+    Levels--;
+    DebugAssert((Levels == 0) || (FKeyHistory.back().Levels == 1));
+  }
+}
+//---------------------------------------------------------------------------
+void __fastcall THierarchicalStorage::CloseSubKey()
+{
+  if (FKeyHistory.empty())
   {
-    FKeyHistory->Delete(FKeyHistory->Count - 1);
+    throw Exception(UnicodeString());
   }
+
+  DebugAssert(FKeyHistory.back().Levels == 1);
+  FKeyHistory.pop_back();
   DoCloseSubKey();
 }
 //---------------------------------------------------------------------------
@@ -211,7 +243,7 @@ void __fastcall THierarchicalStorage::CloseAll()
 {
   while (!CurrentSubKey.IsEmpty())
   {
-    CloseSubKey();
+    CloseSubKeyPath();
   }
 }
 //---------------------------------------------------------------------------
@@ -723,7 +755,7 @@ bool __fastcall TRegistryStorage::DoOpenSubKey(const UnicodeString & SubKey, boo
 void __fastcall TRegistryStorage::DoCloseSubKey()
 {
   FRegistry->CloseKey();
-  if (FKeyHistory->Count > 0)
+  if (!FKeyHistory.empty())
   {
     FRegistry->OpenKey(Storage + GetCurrentSubKeyMunged(), True);
   }
@@ -732,7 +764,7 @@ void __fastcall TRegistryStorage::DoCloseSubKey()
 void __fastcall TRegistryStorage::DoDeleteSubKey(const UnicodeString & SubKey)
 {
   UnicodeString K;
-  if (FKeyHistory->Count == 0)
+  if (FKeyHistory.empty())
   {
     K = Storage + CurrentSubKey;
   }
@@ -946,13 +978,13 @@ bool __fastcall TCustomIniFileStorage::OpenRootKey(bool CanCreate)
   return THierarchicalStorage::OpenRootKey(CanCreate);
 }
 //---------------------------------------------------------------------------
-bool __fastcall TCustomIniFileStorage::OpenSubKey(const UnicodeString & Key, bool CanCreate, bool Path)
+bool __fastcall TCustomIniFileStorage::OpenSubKey(const UnicodeString & Key, bool CanCreate)
 {
   bool Result;
 
   {
     TAutoFlag Flag(FOpeningSubKey);
-    Result = THierarchicalStorage::OpenSubKey(Key, CanCreate, Path);
+    Result = THierarchicalStorage::OpenSubKey(Key, CanCreate);
   }
 
   if (FMasterStorage.get() != NULL)
@@ -963,10 +995,10 @@ bool __fastcall TCustomIniFileStorage::OpenSubKey(const UnicodeString & Key, boo
     }
     else
     {
-      bool MasterResult = FMasterStorage->OpenSubKey(Key, CanCreate, Path);
+      bool MasterResult = FMasterStorage->OpenSubKey(Key, CanCreate);
       if (!Result && MasterResult)
       {
-        Result = THierarchicalStorage::OpenSubKey(Key, true, Path);
+        Result = THierarchicalStorage::OpenSubKey(Key, true);
         DebugAssert(Result);
       }
       else if (Result && !MasterResult)

source/core/HierarchicalStorage.h

@@ -16,9 +16,11 @@ public:
   __fastcall THierarchicalStorage(const UnicodeString & AStorage);
   virtual __fastcall ~THierarchicalStorage();
   virtual bool __fastcall OpenRootKey(bool CanCreate);
-  virtual bool __fastcall OpenSubKey(const UnicodeString & SubKey, bool CanCreate, bool Path = false);
+  virtual bool __fastcall OpenSubKey(const UnicodeString & SubKey, bool CanCreate);
   virtual void __fastcall CloseSubKey();
   void __fastcall CloseAll();
+  bool __fastcall OpenSubKeyPath(const UnicodeString & KeyPath, bool CanCreate);
+  void __fastcall CloseSubKeyPath();
   void __fastcall GetSubKeyNames(TStrings * Strings);
   void __fastcall GetValueNames(TStrings * Strings);
   bool __fastcall HasSubKeys();
@@ -67,8 +69,14 @@ public:
   __property bool Temporary = { read = GetTemporary };
 
 protected:
+  struct TKeyEntry
+  {
+    UnicodeString Key;
+    int Levels;
+  };
+
   UnicodeString FStorage;
-  TStrings * FKeyHistory;
+  std::vector<TKeyEntry> FKeyHistory;
   TStorageAccessMode FAccessMode;
   bool FExplicit;
   bool FForceSave;
@@ -165,7 +173,7 @@ public:
   virtual __fastcall ~TCustomIniFileStorage();
 
   virtual bool __fastcall OpenRootKey(bool CanCreate);
-  virtual bool __fastcall OpenSubKey(const UnicodeString & SubKey, bool CanCreate, bool Path = false);
+  virtual bool __fastcall OpenSubKey(const UnicodeString & SubKey, bool CanCreate);
 
 private:
   UnicodeString __fastcall GetCurrentSection();

source/forms/CustomScpExplorer.cpp

@@ -5510,7 +5510,7 @@ UnicodeString __fastcall TCustomScpExplorerForm::SerializeCopyParamForCommandLin
   TCopyParamType Defaults;
   std::unique_ptr<THierarchicalStorage> ConfigStorage(Configuration->CreateConfigStorage());
   ConfigStorage->AccessMode = smRead;
-  if (ConfigStorage->OpenSubKey(Configuration->ConfigurationSubKey, false, false))
+  if (ConfigStorage->OpenSubKey(Configuration->ConfigurationSubKey, false))
   {
     GUIConfiguration->LoadCopyParam(ConfigStorage.get(), &Defaults);
   }

source/windows/CustomWinConfiguration.cpp

@@ -144,7 +144,7 @@ void __fastcall TCustomWinConfiguration::Saved()
 #define LASTELEM(ELEM) \
   ELEM.SubString(ELEM.LastDelimiter(L".>")+1, ELEM.Length() - ELEM.LastDelimiter(L".>"))
 #define BLOCK(KEY, CANCREATE, BLOCK) \
-  if (Storage->OpenSubKey(KEY, CANCREATE, true)) try { BLOCK } __finally { Storage->CloseSubKey(); }
+  if (Storage->OpenSubKeyPath(KEY, CANCREATE)) try { BLOCK } __finally { Storage->CloseSubKeyPath(); }
 #define REGCONFIG(CANCREATE) \
   BLOCK(L"Interface", CANCREATE, \
     KEY(Integer,  Interface); \

source/windows/GUIConfiguration.cpp

@@ -625,7 +625,7 @@ void __fastcall TGUIConfiguration::UpdateStaticUsage()
 //---------------------------------------------------------------------------
 // duplicated from core\configuration.cpp
 #define BLOCK(KEY, CANCREATE, BLOCK) \
-  if (Storage->OpenSubKey(KEY, CANCREATE, true)) try { BLOCK } __finally { Storage->CloseSubKey(); }
+  if (Storage->OpenSubKeyPath(KEY, CANCREATE)) try { BLOCK } __finally { Storage->CloseSubKeyPath(); }
 #define KEY(TYPE, VAR) KEYEX(TYPE, VAR, PropertyToKey(TEXT(#VAR)))
 #define REGCONFIG(CANCREATE) \
   BLOCK(L"Interface", CANCREATE, \
@@ -657,7 +657,7 @@ void __fastcall TGUIConfiguration::UpdateStaticUsage()
 //---------------------------------------------------------------------------
 bool __fastcall TGUIConfiguration::DoSaveCopyParam(THierarchicalStorage * Storage, const TCopyParamType * CopyParam, const TCopyParamType * Defaults)
 {
-  bool Result = Storage->OpenSubKey(L"Interface\\CopyParam", true, true);
+  bool Result = Storage->OpenSubKeyPath(L"Interface\\CopyParam", true);
   if (Result)
   {
     CopyParam->Save(Storage, Defaults);
@@ -692,24 +692,24 @@ void __fastcall TGUIConfiguration::SaveData(THierarchicalStorage * Storage, bool
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 
-  if (Storage->OpenSubKey(L"Interface\\NewDirectory2", true, true))
+  if (Storage->OpenSubKeyPath(L"Interface\\NewDirectory2", true))
   try
   {
     FNewDirectoryProperties.Save(Storage);
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 }
 //---------------------------------------------------------------------------
 bool __fastcall TGUIConfiguration::LoadCopyParam(THierarchicalStorage * Storage, TCopyParamType * CopyParam)
 {
   bool Result =
-    Storage->OpenSubKey(L"Interface\\CopyParam", false, true);
+    Storage->OpenSubKeyPath(L"Interface\\CopyParam", false);
   if (Result)
   {
     try
@@ -718,7 +718,7 @@ bool __fastcall TGUIConfiguration::LoadCopyParam(THierarchicalStorage * Storage,
     }
     catch (...)
     {
-      Storage->CloseSubKey();
+      Storage->CloseSubKeyPath();
       throw;
     }
   }
@@ -761,7 +761,7 @@ void __fastcall TGUIConfiguration::LoadData(THierarchicalStorage * Storage)
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 
   // Make it compatible with versions prior to 3.7.1 that have not saved PuttyPath
@@ -778,14 +778,14 @@ void __fastcall TGUIConfiguration::LoadData(THierarchicalStorage * Storage)
     FPuttyPath = FormatCommand(FPuttyPath, L"");
   }
 
-  if (Storage->OpenSubKey(L"Interface\\NewDirectory2", false, true))
+  if (Storage->OpenSubKeyPath(L"Interface\\NewDirectory2", false))
   try
   {
     FNewDirectoryProperties.Load(Storage);
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 }
 //---------------------------------------------------------------------------

source/windows/Setup.cpp

@@ -2222,7 +2222,7 @@ UnicodeString __fastcall GetPowerShellVersionStr()
       for (int Index = 0; Index < Keys->Count; Index++)
       {
         UnicodeString Key = Keys->Strings[Index];
-        if (Registry->OpenSubKey(Key + L"\\PowerShellEngine", false, true))
+        if (Registry->OpenSubKeyPath(Key + L"\\PowerShellEngine", false))
         {
           UnicodeString VersionStr = Registry->ReadString(L"PowerShellVersion", L"");
           if (!VersionStr.IsEmpty() && (CompareVersion(VersionStr, PowerShellVersionStr) > 0))
@@ -2230,7 +2230,7 @@ UnicodeString __fastcall GetPowerShellVersionStr()
             PowerShellVersionStr = VersionStr;
           }
 
-          Registry->CloseSubKey();
+          Registry->CloseSubKeyPath();
         }
       }
     }
@@ -2245,7 +2245,7 @@ static void CollectCLSIDKey(
   UnicodeString & CommonCodeBase, const UnicodeString & Platform, UnicodeString & Platforms)
 {
   UnicodeString CLSIDKey = FORMAT(L"CLSID\\%s", (CLSID));
-  if (Storage->OpenSubKey(CLSIDKey, false, true))
+  if (Storage->OpenSubKeyPath(CLSIDKey, false))
   {
     int Index = Keys->IndexOf(CLSIDKey);
     if (Index >= 0)
@@ -2292,7 +2292,7 @@ static void CollectCLSIDKey(
       }
       Storage->CloseSubKey();
     }
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
 
     UnicodeString Buf = Platform;
     AddToList(Buf, CodeBase, L" - ");
@@ -2345,7 +2345,7 @@ static void DoCollectComRegistration(TConsole * Console, TStrings * Keys)
   {
     Console->PrintLine(FORMAT(L"Versions of type library %s:", (TypeLib)));
     UnicodeString TypeLibKey = FORMAT(L"TypeLib\\%s", (TypeLib));
-    if (Storage->OpenSubKey(TypeLibKey, false, true))
+    if (Storage->OpenSubKeyPath(TypeLibKey, false))
     {
       Keys->Add(TypeLibKey);
       std::unique_ptr<TStringList> KeyNames(new TStringList());
@@ -2359,7 +2359,7 @@ static void DoCollectComRegistration(TConsole * Console, TStrings * Keys)
         for (int Index = 0; Index < KeyNames->Count; Index++)
         {
           UnicodeString Version = KeyNames->Strings[Index];
-          if (!Storage->OpenSubKey(FORMAT(L"%s\\0", (Version)), false, true))
+          if (!Storage->OpenSubKeyPath(FORMAT(L"%s\\0", (Version)), false))
           {
             Console->PrintLine(FORMAT(L"Warning: Subkey \"0\" for type library \"%s\" cannot be opened.", (Version)));
           }
@@ -2389,11 +2389,11 @@ static void DoCollectComRegistration(TConsole * Console, TStrings * Keys)
                 }
               }
             }
-            Storage->CloseSubKey();
+            Storage->CloseSubKeyPath();
           }
         }
       }
-      Storage->CloseSubKey();
+      Storage->CloseSubKeyPath();
     }
     else
     {
@@ -2412,11 +2412,11 @@ static void DoCollectComRegistration(TConsole * Console, TStrings * Keys)
       UnicodeString KeyName = KeyNames->Strings[Index];
       if (StartsText(NamespacePrefix, KeyName))
       {
-        if (Storage->OpenSubKey(FORMAT(L"%s\\%s", (KeyName, L"CLSID")), false, true))
+        if (Storage->OpenSubKeyPath(FORMAT(L"%s\\%s", (KeyName, L"CLSID")), false))
         {
           UnicodeString Class = KeyName;
           UnicodeString CLSID = Trim(Storage->ReadString(UnicodeString(), UnicodeString()));
-          Storage->CloseSubKey();
+          Storage->CloseSubKeyPath();
 
           if (!CLSID.IsEmpty())
           {
@@ -2488,11 +2488,11 @@ static void DoCollectComRegistration(TConsole * Console, TStrings * Keys)
         // Open sub key first, to check if we are interested in the interface, as an optimization
         int PlatformSet = reinterpret_cast<int>(KeyNames->Objects[Index]);
         THierarchicalStorage * KeyStorage = FLAGSET(PlatformSet, 32) ? Storage.get() : Storage64.get();
-        if (KeyStorage->OpenSubKey(FORMAT(L"%s\\TypeLib", (KeyName)), false, true))
+        if (KeyStorage->OpenSubKeyPath(FORMAT(L"%s\\TypeLib", (KeyName)), false))
         {
           UnicodeString InterfaceTypeLib = KeyStorage->ReadString(UnicodeString(), UnicodeString());
           UnicodeString Version = KeyStorage->ReadString(L"Version", UnicodeString());
-          KeyStorage->CloseSubKey();
+          KeyStorage->CloseSubKeyPath();
           if (SameText(InterfaceTypeLib, TypeLib))
           {
             if (KeyStorage->OpenSubKey(KeyName, false))

source/windows/WinConfiguration.cpp

@@ -940,7 +940,7 @@ THierarchicalStorage * TWinConfiguration::CreateScpStorage(bool & SessionList)
 #define LASTELEM(ELEM) \
   ELEM.SubString(ELEM.LastDelimiter(L".>")+1, ELEM.Length() - ELEM.LastDelimiter(L".>"))
 #define BLOCK(KEY, CANCREATE, BLOCK) \
-  if (Storage->OpenSubKey(KEY, CANCREATE, true)) try { BLOCK } __finally { Storage->CloseSubKey(); }
+  if (Storage->OpenSubKeyPath(KEY, CANCREATE)) try { BLOCK } __finally { Storage->CloseSubKeyPath(); }
 #define KEY(TYPE, VAR) KEYEX(TYPE, VAR, PropertyToKey(TEXT(#VAR)))
 #define REGCONFIG(CANCREATE) \
   BLOCK(L"Interface", CANCREATE, \
@@ -1182,14 +1182,14 @@ void __fastcall TWinConfiguration::SaveData(THierarchicalStorage * Storage, bool
   }
 
   if ((All || FEditorList->Modified) &&
-      Storage->OpenSubKey(L"Interface\\Editor", true, true))
+      Storage->OpenSubKeyPath(L"Interface\\Editor", true))
   try
   {
     FEditorList->Save(Storage);
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 }
 //---------------------------------------------------------------------------
@@ -1524,7 +1524,7 @@ void __fastcall TWinConfiguration::LoadData(THierarchicalStorage * Storage)
     FCustomCommandOptionsModified = false;
   }
 
-  if (Storage->OpenSubKey(L"Interface\\Editor", false, true))
+  if (Storage->OpenSubKeyPath(L"Interface\\Editor", false))
   try
   {
     FEditorList->Clear();
@@ -1532,12 +1532,12 @@ void __fastcall TWinConfiguration::LoadData(THierarchicalStorage * Storage)
   }
   __finally
   {
-    Storage->CloseSubKey();
+    Storage->CloseSubKeyPath();
   }
 
   // load legacy editor configuration
   DebugAssert(FLegacyEditor != NULL);
-  if (Storage->OpenSubKey(L"Interface\\Editor", false, true))
+  if (Storage->OpenSubKeyPath(L"Interface\\Editor", false))
   {
     try
     {
@@ -1545,7 +1545,7 @@ void __fastcall TWinConfiguration::LoadData(THierarchicalStorage * Storage)
     }
     __finally
     {
-      Storage->CloseSubKey();
+      Storage->CloseSubKeyPath();
     }
   }
 }

source/windows/WinMain.cpp

@@ -349,7 +349,7 @@ void __fastcall RecordWrapperVersions(UnicodeString ConsoleVersion, UnicodeStrin
       {
         Storage->AccessMode = smReadWrite;
         if (Storage->OpenSubKey(Configuration->ConfigurationSubKey, true) &&
-            Storage->OpenSubKey(L"Interface\\Updates", true, true))
+            Storage->OpenSubKeyPath(L"Interface\\Updates", true))
         {
           if (!DotNetVersion.IsEmpty())
           {