Some (hopefully noop) refactoring to solve Clang internal compiler error when using new variable in __finally block + Some other small code changes imposed by Clang

Source commit: eef61854e4463990c6e823a08451a9e229a8c54b

source/core/Common.cpp

@@ -3246,7 +3246,7 @@ UnicodeString __fastcall FormatSize(__int64 Size)
 UnicodeString FormatDateTimeSpan(const TDateTime & DateTime)
 {
   UnicodeString Result;
-  if ((0 <= DateTime) && (DateTime <= MaxDateTime))
+  if ((TDateTime() <= DateTime) && (DateTime <= MaxDateTime))
   {
     TTimeStamp TimeStamp = DateTimeToTimeStamp(DateTime);
     int Days = TimeStamp.Date - DateDelta;

source/core/PuttyIntf.cpp

@@ -1177,7 +1177,7 @@ UnicodeString CalculateFileChecksum(TStream * Stream, const UnicodeString & Alg)
     throw Exception(FMTLOAD(UNKNOWN_CHECKSUM, (Alg)));
   }
 
-  UnicodeString Result;
+  RawByteString Buf;
   ssh_hash * Hash = ssh_hash_new(HashAlg);
   try
   {
@@ -1197,12 +1197,12 @@ UnicodeString CalculateFileChecksum(TStream * Stream, const UnicodeString & Alg)
   }
   __finally
   {
-    RawByteString Buf;
     Buf.SetLength(ssh_hash_alg(Hash)->hlen);
     ssh_hash_final(Hash, reinterpret_cast<unsigned char *>(Buf.c_str()));
-    Result = BytesToHex(Buf);
   }
 
+  UnicodeString Result = BytesToHex(Buf);
+
   return Result;
 }
 //---------------------------------------------------------------------------

source/core/Queue.cpp

@@ -2615,6 +2615,11 @@ void __fastcall TTerminalThread::TerminalReopen()
   RunAction(TerminalReopenEvent);
 }
 //---------------------------------------------------------------------------
+void TTerminalThread::DiscardException()
+{
+  SAFE_DESTROY(FException);
+}
+//---------------------------------------------------------------------------
 void __fastcall TTerminalThread::RunAction(TNotifyEvent Action)
 {
   DebugAssert(FAction == NULL);
@@ -2694,7 +2699,7 @@ void __fastcall TTerminalThread::RunAction(TNotifyEvent Action)
     __finally
     {
       FAction = NULL;
-      SAFE_DESTROY(FException);
+      DiscardException();
     }
   }
   catch(...)
@@ -2877,7 +2882,7 @@ void __fastcall TTerminalThread::WaitForUserAction(TUserAction * UserAction)
     __finally
     {
       FUserAction = PrevUserAction;
-      SAFE_DESTROY(FException);
+      DiscardException();
     }
 
     // Contrary to a call before, this is unconditional,

source/core/Queue.h

@@ -487,6 +487,7 @@ private:
   void __fastcall TerminalStartReadDirectory(TObject * Sender);
   void __fastcall TerminalReadDirectoryProgress(TObject * Sender, int Progress, int ResolvedLinks, bool & Cancel);
   void __fastcall TerminalInitializeLog(TObject * Sender);
+  void DiscardException();
 };
 //---------------------------------------------------------------------------
 enum TQueueFileState { qfsQueued = 0, qfsProcessed = 1 };

source/core/ScpFileSystem.cpp

@@ -553,7 +553,7 @@ bool __fastcall TSCPFileSystem::RemoveLastLine(UnicodeString & Line,
   return IsLastLine;
 }
 //---------------------------------------------------------------------------
-bool __fastcall TSCPFileSystem::IsLastLine(UnicodeString & Line)
+bool __fastcall TSCPFileSystem::TryRemoveLastLine(UnicodeString & Line)
 {
   bool Result = false;
   try
@@ -567,6 +567,12 @@ bool __fastcall TSCPFileSystem::IsLastLine(UnicodeString & Line)
   return Result;
 }
 //---------------------------------------------------------------------------
+bool TSCPFileSystem::IsLastLine(const UnicodeString & ALine)
+{
+  UnicodeString Line = ALine;
+  return TryRemoveLastLine(Line);
+}
+//---------------------------------------------------------------------------
 void __fastcall TSCPFileSystem::SkipFirstLine()
 {
   UnicodeString Line = FSecureShell->ReceiveLine();
@@ -590,7 +596,7 @@ void __fastcall TSCPFileSystem::ReadCommandOutput(int Params, const UnicodeStrin
       do
       {
         Line = FSecureShell->ReceiveLine();
-        IsLast = IsLastLine(Line);
+        IsLast = TryRemoveLastLine(Line);
         if (!IsLast || !Line.IsEmpty())
         {
           FOutput->Add(Line);
@@ -1620,7 +1626,7 @@ void __fastcall TSCPFileSystem::SCPResponse(bool * GotLastLine)
       // pscp adds 'Resp' to 'Msg', why?
       UnicodeString Msg = FSecureShell->ReceiveLine();
       UnicodeString Line = UnicodeString(static_cast<char>(Resp)) + Msg;
-      if (IsLastLine(Line))
+      if (TryRemoveLastLine(Line))
       {
         if (GotLastLine != NULL)
         {
@@ -2398,23 +2404,21 @@ void __fastcall TSCPFileSystem::CopyToLocal(TStrings * FilesToCopy,
         (OperationProgress->Cancel == csCancel) ||
         (OperationProgress->Cancel == csCancelTransfer)))
     {
-      bool LastLineRead;
-
       // If we get LastLine, it means that remote side 'scp' is already
       // terminated, so we need not to terminate it. There is also
       // possibility that remote side waits for confirmation, so it will hang.
       // This should not happen (hope)
-      UnicodeString Line = FSecureShell->ReceiveLine();
-      LastLineRead = IsLastLine(Line);
-      if (!LastLineRead)
+      if (!IsLastLine(FSecureShell->ReceiveLine()))
       {
         SCPSendError((OperationProgress->Cancel ? L"Terminated by user." : L"Exception"), true);
+        // Just in case, remote side already sent some more data (it's probable)
+        // but we don't want to raise exception (user asked to terminate, it's not error)
+        ReadCommandOutput(coOnlyReturnCode | coWaitForLastLine);
+      }
+      else
+      {
+        ReadCommandOutput(coOnlyReturnCode);
       }
-      // Just in case, remote side already sent some more data (it's probable)
-      // but we don't want to raise exception (user asked to terminate, it's not error)
-      int ECParams = coOnlyReturnCode;
-      if (!LastLineRead) ECParams |= coWaitForLastLine;
-      ReadCommandOutput(ECParams);
     }
   }
 }
@@ -2488,7 +2492,7 @@ void __fastcall TSCPFileSystem::SCPSink(const UnicodeString TargetDir,
 
       if (Line.Length() == 0) FTerminal->FatalError(NULL, LoadStr(SCP_EMPTY_LINE));
 
-      if (IsLastLine(Line))
+      if (TryRemoveLastLine(Line))
       {
         // Remote side finished copying, so remote SCP was closed
         // and we don't need to terminate it manually, see CopyToLocal()

source/core/ScpFileSystem.h

@@ -111,7 +111,8 @@ private:
   void __fastcall CustomReadFile(const UnicodeString FileName,
     TRemoteFile *& File, TRemoteFile * ALinkedByFile);
   void __fastcall DetectReturnVar();
-  bool __fastcall IsLastLine(UnicodeString & Line);
+  bool __fastcall TryRemoveLastLine(UnicodeString & Line);
+  bool IsLastLine(const UnicodeString & ALine);
   static bool __fastcall IsTotalListingLine(const UnicodeString Line);
   void __fastcall EnsureLocation();
   void __fastcall ExecCommand(TFSCommand Cmd, const TVarRec * args = NULL,

source/core/Script.cpp

@@ -579,7 +579,7 @@ TStrings * __fastcall TScript::CreateFileList(TScriptProcParams * Parameters, in
   TStrings * Result = new TStringList();
   try
   {
-    TStrings * FileLists = NULL;
+    TStringList * FileLists = NULL;
     try
     {
       for (int i = Start; i <= End; i++)
@@ -621,6 +621,7 @@ TStrings * __fastcall TScript::CreateFileList(TScriptProcParams * Parameters, in
             if (FileLists == NULL)
             {
               FileLists = new TStringList();
+              FileLists->OwnsObjects = true;
             }
             FileLists->AddObject(Directory, FileList);
           }
@@ -678,14 +679,7 @@ TStrings * __fastcall TScript::CreateFileList(TScriptProcParams * Parameters, in
     }
     __finally
     {
-      if (FileLists != NULL)
-      {
-        for (int i = 0; i < FileLists->Count; i++)
-        {
-          delete FileLists->Objects[i];
-        }
-        delete FileLists;
-      }
+      delete FileLists;
     }
 
     if (FLAGSET(ListType, fltLatest) && (Result->Count > 1))

source/core/Terminal.cpp

@@ -1714,7 +1714,7 @@ void __fastcall TTerminal::OpenTunnel()
       }
       int Index = Random(Ports.size());
       int Port = Ports[Index];
-      Ports.erase(&Ports.at(Index));
+      Ports.erase(Ports.begin() + Index);
       if (IsListenerFree(Port))
       {
         FTunnelLocalPortNumber = Port;
@@ -3527,6 +3527,25 @@ void __fastcall TTerminal::ReadCurrentDirectory()
   }
 }
 //---------------------------------------------------------------------------
+void TTerminal::DoReadDirectoryFinish(TRemoteDirectory * Files, bool ReloadOnly)
+{
+  // Factored out to solve Clang ICE
+  TRemoteDirectory * OldFiles = FFiles;
+  FFiles = Files;
+  try
+  {
+    DoReadDirectory(ReloadOnly);
+  }
+  __finally
+  {
+    // delete only after loading new files to dir view,
+    // not to destroy the file objects that the view holds
+    // (can be issue in multi threaded environment, such as when the
+    // terminal is reconnecting in the terminal thread)
+    delete OldFiles;
+  }
+}
+//---------------------------------------------------------------------------
 void __fastcall TTerminal::ReadDirectory(bool ReloadOnly, bool ForceCache)
 {
   bool LoadedFromCache = false;
@@ -3579,20 +3598,7 @@ void __fastcall TTerminal::ReadDirectory(bool ReloadOnly, bool ForceCache)
       {
         DoReadDirectoryProgress(-1, 0, Cancel);
         FReadingCurrentDirectory = false;
-        TRemoteDirectory * OldFiles = FFiles;
-        FFiles = Files;
-        try
-        {
-          DoReadDirectory(ReloadOnly);
-        }
-        __finally
-        {
-          // delete only after loading new files to dir view,
-          // not to destroy the file objects that the view holds
-          // (can be issue in multi threaded environment, such as when the
-          // terminal is reconnecting in the terminal thread)
-          delete OldFiles;
-        }
+        DoReadDirectoryFinish(Files, ReloadOnly);
         if (Active)
         {
           if (SessionData->CacheDirectories)
@@ -6097,6 +6103,20 @@ bool __fastcall TTerminal::IsEmptyLocalDirectory(
   return Contents.IsEmpty();
 }
 //---------------------------------------------------------------------------
+void DestroyLocalFileList(TStringList * LocalFileList)
+{
+  // Factored out to workaround Clang ICE
+  if (LocalFileList != NULL)
+  {
+    for (int Index = 0; Index < LocalFileList->Count; Index++)
+    {
+      TSynchronizeFileData * FileData = reinterpret_cast<TSynchronizeFileData*>(LocalFileList->Objects[Index]);
+      delete FileData;
+    }
+    delete LocalFileList;
+  }
+}
+//---------------------------------------------------------------------------
 void __fastcall TTerminal::DoSynchronizeCollectDirectory(const UnicodeString LocalDirectory,
   const UnicodeString RemoteDirectory, TSynchronizeMode Mode,
   const TCopyParamType * CopyParam, int Params,
@@ -6266,16 +6286,7 @@ void __fastcall TTerminal::DoSynchronizeCollectDirectory(const UnicodeString Loc
   }
   __finally
   {
-    if (Data.LocalFileList != NULL)
-    {
-      for (int Index = 0; Index < Data.LocalFileList->Count; Index++)
-      {
-        TSynchronizeFileData * FileData = reinterpret_cast<TSynchronizeFileData*>
-          (Data.LocalFileList->Objects[Index]);
-        delete FileData;
-      }
-      delete Data.LocalFileList;
-    }
+    DestroyLocalFileList(Data.LocalFileList);
   }
 }
 //---------------------------------------------------------------------------
@@ -7421,10 +7432,9 @@ bool __fastcall TTerminal::CopyToRemote(
       {
         if (Configuration->Usage->Collect)
         {
-          int CounterSize = TUsage::CalculateCounterSize(Size);
           Configuration->Usage->Inc(L"Uploads");
-          Configuration->Usage->Inc(L"UploadedBytes", CounterSize);
-          Configuration->Usage->SetMax(L"MaxUploadSize", CounterSize);
+          int CounterSize = TUsage::CalculateCounterSize(Size);
+          Configuration->Usage->IncAndSetMax(L"UploadedBytes", L"MaxUploadSize", CounterSize);
           CollectingUsage = true;
         }
 
@@ -7469,9 +7479,7 @@ bool __fastcall TTerminal::CopyToRemote(
     {
       if (CollectingUsage)
       {
-        int CounterTime = TimeToSeconds(OperationProgress.TimeElapsed());
-        Configuration->Usage->Inc(L"UploadTime", CounterTime);
-        Configuration->Usage->SetMax(L"MaxUploadTime", CounterTime);
+        Configuration->Usage->IncAndSetMax(L"UploadTime", L"MaxUploadTime", TimeToSeconds(OperationProgress.TimeElapsed()));
       }
       OperationStop(OperationProgress);
     }
@@ -8001,8 +8009,7 @@ bool __fastcall TTerminal::CopyToLocal(
         {
           int CounterTotalSize = TUsage::CalculateCounterSize(TotalSize);
           Configuration->Usage->Inc(L"Downloads");
-          Configuration->Usage->Inc(L"DownloadedBytes", CounterTotalSize);
-          Configuration->Usage->SetMax(L"MaxDownloadSize", CounterTotalSize);
+          Configuration->Usage->IncAndSetMax(L"DownloadedBytes", L"MaxDownloadSize", CounterTotalSize);
           CollectingUsage = true;
         }
 
@@ -8068,9 +8075,7 @@ bool __fastcall TTerminal::CopyToLocal(
     {
       if (CollectingUsage)
       {
-        int CounterTime = TimeToSeconds(OperationProgress.TimeElapsed());
-        Configuration->Usage->Inc(L"DownloadTime", CounterTime);
-        Configuration->Usage->SetMax(L"MaxDownloadTime", CounterTime);
+        Configuration->Usage->IncAndSetMax(L"DownloadTime", L"MaxDownloadTime", TimeToSeconds(OperationProgress.TimeElapsed()));
       }
       OperationStop(OperationProgress);
     }

source/core/Terminal.h

@@ -277,6 +277,7 @@ protected:
   void __fastcall DoStartReadDirectory();
   void __fastcall DoReadDirectoryProgress(int Progress, int ResolvedLinks, bool & Cancel);
   void __fastcall DoReadDirectory(bool ReloadOnly);
+  void DoReadDirectoryFinish(TRemoteDirectory * Files, bool ReloadOnly);
   void __fastcall DoCreateDirectory(const UnicodeString & DirName, bool Encrypt);
   void __fastcall DoDeleteFile(
     TCustomFileSystem * FileSystem, const UnicodeString & FileName, const TRemoteFile * File, int Params);

source/core/Usage.cpp

@@ -256,6 +256,12 @@ void __fastcall TUsage::SetMax(const UnicodeString & Key, int Value)
   }
 }
 //---------------------------------------------------------------------------
+void __fastcall TUsage::IncAndSetMax(const UnicodeString & IncKey, const UnicodeString & MaxKey, int Value)
+{
+  Inc(IncKey, Value);
+  SetMax(MaxKey, Value);
+}
+//---------------------------------------------------------------------------
 void __fastcall TUsage::SetMax(const UnicodeString & Key, int Value,
   TCounters & Counters)
 {

source/core/Usage.h

@@ -18,6 +18,7 @@ public:
   void __fastcall Set(const UnicodeString & Key, bool Value);
   int __fastcall Inc(const UnicodeString & Key, int Increment = 1);
   void __fastcall SetMax(const UnicodeString & Key, int Value);
+  void __fastcall IncAndSetMax(const UnicodeString & IncKey, const UnicodeString & MaxKey, int Value);
   UnicodeString __fastcall Get(const UnicodeString & Key);
 
   void __fastcall UpdateCurrentVersion();

source/filezilla/FileZillaApi.cpp

@@ -416,7 +416,7 @@ int CFileZillaApi::Rename(CString oldName, CString newName, const CServerPath &p
 
 int CFileZillaApi::SetAsyncRequestResult(int nAction, CAsyncRequestData *pData)
 {
-  if (!this || !pData)
+  if (!pData)
     return FZ_REPLY_CRITICALERROR | FZ_REPLY_INVALIDPARAM;
 
   if (IsBadWritePtr(pData, sizeof(CAsyncRequestData)))

source/filezilla/ServerPath.cpp

@@ -417,9 +417,6 @@ const bool CServerPath::operator==(const CServerPath &op) const
 
 const bool CServerPath::operator!=(const CServerPath &op) const
 {
-  if (!this)
-    return false;
-
   if (*this == op)
     return false;
   else