Code errors identified by Clang

Notably:
Bug fix: Monospace font is not remembered if SSH banners are administratively forced
Change: Bug fix: In XML log, filename tag was included in stat tag, although explicitly documented not to

Source commit: 693ecd9b083f1460dc4d1ae1ff49e625989f8a50

source/core/Common.cpp

@@ -34,7 +34,7 @@ const UnicodeString AnyMask = L"*.*";
 const wchar_t EngShortMonthNames[12][4] =
   {L"Jan", L"Feb", L"Mar", L"Apr", L"May", L"Jun",
    L"Jul", L"Aug", L"Sep", L"Oct", L"Nov", L"Dec"};
-const char Bom[3] = "\xEF\xBB\xBF";
+const char Bom[4] = "\xEF\xBB\xBF";
 const UnicodeString XmlDeclaration(TraceInitStr(L"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"));
 const wchar_t TokenPrefix = L'%';
 const wchar_t NoReplacement = wchar_t(false);

source/core/Common.h

@@ -23,7 +23,7 @@
 //---------------------------------------------------------------------------
 extern const UnicodeString AnyMask;
 extern const wchar_t EngShortMonthNames[12][4];
-extern const char Bom[3];
+extern const char Bom[4];
 extern const UnicodeString XmlDeclaration;
 extern const wchar_t TokenPrefix;
 extern const wchar_t NoReplacement;

source/core/FileBuffer.cpp

@@ -105,7 +105,7 @@ void __fastcall TFileBuffer::Convert(char * Source, char * Dest, int Params,
   DebugAssert(strlen(Dest) <= 2);
 
   if (FLAGSET(Params, cpRemoveBOM) && (Size >= 3) &&
-      (memcmp(Data, Bom, sizeof(Bom)) == 0))
+      (memcmp(Data, Bom, strlen(Bom)) == 0))
   {
     Delete(0, 3);
   }

source/core/FtpFileSystem.cpp

@@ -2314,7 +2314,7 @@ void __fastcall TFTPFileSystem::AutoDetectTimeDifference(
       // do we need FTimeDifference for the operation?
       // (tmAutomatic - AsciiFileMask can theoretically include time constraints, while it is unlikely)
       (!FLAGSET(Params, cpNoConfirmation) ||
-       CopyParam->NewerOnly || (!CopyParam->TransferMode == tmAutomatic) || !CopyParam->IncludeFileMask.Masks.IsEmpty()))
+       CopyParam->NewerOnly || (CopyParam->TransferMode == tmAutomatic) || !CopyParam->IncludeFileMask.Masks.IsEmpty()))
   {
     FTerminal->LogEvent(L"Retrieving listing to detect timezone difference");
     DummyReadDirectory(Directory);

source/core/HierarchicalStorage.cpp

@@ -69,9 +69,9 @@ UnicodeString __fastcall UnMungeStr(const UnicodeString & Str)
   RawByteString Dest(sb->s);
   strbuf_free(sb);
   UnicodeString Result;
-  if (Dest.SubString(1, LENOF(Bom)) == Bom)
+  if (Dest.SubString(1, strlen(Bom)) == Bom)
   {
-    Dest.Delete(1, LENOF(Bom));
+    Dest.Delete(1, strlen(Bom));
     Result = UTF8ToString(Dest);
   }
   else

source/core/S3FileSystem.cpp

@@ -1858,7 +1858,8 @@ static _di_IXMLNode S3NeedNode(const _di_IXMLNodeList & NodeList, const UnicodeS
 //---------------------------------------------------------------------------
 #define COPY_BUCKET_CONTEXT(BucketContext) \
   { BucketContext.hostName, BucketContext.bucketName, BucketContext.protocol, BucketContext.uriStyle, \
-    BucketContext.accessKeyId, BucketContext.secretAccessKey, BucketContext.securityToken, BucketContext.authRegion }
+    BucketContext.accessKeyId, BucketContext.secretAccessKey, BucketContext.securityToken, BucketContext.authRegion, \
+    BucketContext.service }
 //---------------------------------------------------------------------------
 bool TS3FileSystem::DoLoadFileProperties(
   const UnicodeString & AFileName, const TRemoteFile * File, TS3FileProperties & Properties, bool LoadTags)
@@ -2155,7 +2156,7 @@ void __fastcall TS3FileSystem::LoadFileProperties(const UnicodeString AFileName,
   Data.Result = DoLoadFileProperties(AFileName, File, Properties, Data.LoadTags);
   if (Data.Result)
   {
-    bool AdditionalRights;
+    bool AdditionalRights = false;
     unsigned short Permissions = 0;
     for (int Index = 0; Index < Properties.AclGrantCount; Index++)
     {

source/core/ScpFileSystem.cpp

@@ -2616,8 +2616,8 @@ void __fastcall TSCPFileSystem::SCPSink(const UnicodeString TargetDir,
         if (CopyParam->SkipTransfer(FullFileName, Dir))
         {
           SkipConfirmed = true;
-          SCPError(L"", false);
           OperationProgress->AddSkippedFileSize(MaskParams.Size);
+          SCPError(L"", false);
         }
 
         FTerminal->LogFileDetails(FileName, FileData.Modification, MaskParams.Size);

source/core/Script.cpp

@@ -2752,6 +2752,7 @@ void __fastcall TManagementScript::Connect(const UnicodeString Session,
       {
         Data = Data->Clone();
       }
+      DefaultsOnly = false;
     }
     else
     {

source/core/SessionInfo.cpp

@@ -414,7 +414,10 @@ protected:
   void __fastcall RecordFile(const UnicodeString & Indent, TRemoteFile * File, bool IncludeFileName)
   {
     FLog->AddIndented(Indent + L"<file>");
-    FLog->AddIndented(Indent + FORMAT(L"  <filename value=\"%s\" />", (XmlAttributeEscape(File->FileName))));
+    if (IncludeFileName)
+    {
+      FLog->AddIndented(Indent + FORMAT(L"  <filename value=\"%s\" />", (XmlAttributeEscape(File->FileName))));
+    }
     FLog->AddIndented(Indent + FORMAT(L"  <type value=\"%s\" />", (XmlAttributeEscape(towupper(File->Type)))));
     if (!File->IsDirectory)
     {

source/core/SftpFileSystem.cpp

@@ -1758,7 +1758,7 @@ protected:
         (FFileSystem->FSecureShell->SshImplementation == sshiBitvise) ||
         (FFileSystem->FSupport->Loaded &&
          FLAGSET(FFileSystem->FSupport->AttributeMask, SSH_FILEXFER_ATTR_OWNERGROUP) &&
-         !File->Owner.IsSet || !File->Group.IsSet);
+         (!File->Owner.IsSet || !File->Group.IsSet));
 
       Result = (MissingRights || MissingOwnerGroup);
       if (Result)

source/core/Terminal.cpp

@@ -2097,14 +2097,14 @@ void __fastcall TTerminal::DisplayBanner(const UnicodeString & Banner)
 {
   if (OnDisplayBanner != NULL)
   {
-    unsigned int OrigParams, Params;
-    if (Configuration->ForceBanners ||
-        Configuration->ShowBanner(SessionData->SessionKey, Banner, Params))
+    unsigned int Params = 0; // shut up
+    if (Configuration->ShowBanner(SessionData->SessionKey, Banner, Params) ||
+        Configuration->ForceBanners)
     {
       bool NeverShowAgain = false;
       int Options =
         FLAGMASK(Configuration->ForceBanners, boDisableNeverShowAgain);
-      OrigParams = Params;
+      unsigned int OrigParams = Params;
 
       TCallbackGuard Guard(this);
       try

source/filezilla/FtpControlSocket.cpp

@@ -5585,7 +5585,7 @@ void CFtpControlSocket::SetVerifyCertResult(int nResult, t_SslCertData *pData)
   DebugAssert(pData);
   if (!m_pSslLayer)
     return;
-  if (!m_Operation.nOpMode == CSMODE_CONNECT)
+  if (DebugAlwaysFalse(m_Operation.nOpMode != CSMODE_CONNECT))
     return;
   m_bCheckForTimeout = TRUE;
   m_pSslLayer->SetNotifyReply(pData->priv_data, SSL_VERIFY_CERT, nResult);

source/filezilla/FtpListResult.cpp

@@ -2073,7 +2073,7 @@ BOOL CFtpListResult::parseAsOther(const char *line, const int linelen, t_directo
         {
           direntry.dir = TRUE;
           tokenlen -= 5;
-          while (tokenlen && str[tokenlen - 1] == ' ' || str[tokenlen - 1] == '\t')
+          while (tokenlen && (str[tokenlen - 1] == ' ' || str[tokenlen - 1] == '\t'))
             tokenlen--;
         }
         if (!tokenlen)

source/filezilla/TransferSocket.cpp

@@ -1140,14 +1140,15 @@ int CTransferSocket::ReadDataFromFile(char *buffer, int len)
   {
     // Comparing to Filezilla 2, we do not do any translation locally,
     // leaving it onto the server (what Filezilla 3 seems to do too)
-    const char Bom[3] = "\xEF\xBB\xBF";
+    const char Bom[4] = "\xEF\xBB\xBF";
+    int BomLen = strlen(Bom);
     int read = ReadData(buffer, len);
     if (GetOptionVal(OPTION_MPEXT_REMOVE_BOM) &&
-        m_transferdata.bType && (read >= sizeof(Bom)) && (memcmp(buffer, Bom, sizeof(Bom)) == 0))
+        m_transferdata.bType && (read >= BomLen) && (memcmp(buffer, Bom, BomLen) == 0))
     {
-      memcpy(buffer, buffer + sizeof(Bom), read - sizeof(Bom));
-      read -= sizeof(Bom);
-      int read2 = ReadData(buffer + read, sizeof(Bom));
+      memcpy(buffer, buffer + BomLen, read - BomLen);
+      read -= BomLen;
+      int read2 = ReadData(buffer + read, BomLen);
       if (read2 > 0)
       {
         read += read2;

source/putty/errsock.c

@@ -43,6 +43,7 @@ static const SocketVtable ErrorSocket_sockvt = {
     // WINSCP
     /*.plug =*/ sk_error_plug,
     /*.close =*/ sk_error_close,
+    NULL, NULL, NULL, NULL,
     /*.socket_error =*/ sk_error_socket_error,
     /*.endpoint_info =*/ nullsock_endpoint_info,
     /* other methods are NULL */

source/putty/windows/gss.c

@@ -273,7 +273,7 @@ struct ssh_gss_liblist *ssh_gss_setup(Conf *conf, LogContext *logctx) // MPEXT
                                 LOAD_LIBRARY_SEARCH_USER_DIRS);
         // MPEXT
         if (!module && logctx) {
-            char *buf = dupprintf("Cannot load GSSAPI from user-specified library '%s': %s", path, win_strerror(GetLastError()));
+            char *buf = dupprintf(WINSCP_BOM "Cannot load GSSAPI from user-specified library '%s': %s", filename_to_str(customlib), win_strerror(GetLastError()));
             logevent(logctx, buf);
             sfree(buf);
         }

source/putty/windows/network.c

@@ -930,6 +930,7 @@ static DWORD try_connect(NetSocket *sock,
     int family;
 #ifdef MPEXT
     struct timeval rcvtimeo;
+    int optlen = sizeof(rcvtimeo);
 #endif
 
     if (sock->s != INVALID_SOCKET) {
@@ -1098,7 +1099,8 @@ static DWORD try_connect(NetSocket *sock,
 #ifdef MPEXT
     if (timeout > 0)
     {
-        if (p_getsockopt (s, SOL_SOCKET, SO_RCVTIMEO, (char *)&rcvtimeo, &rcvtimeo) < 0)
+        // Actually, on Windows SO_RCVTIMEO uses int, so only rcvtimeo.tv_sec is used
+        if (p_getsockopt (s, SOL_SOCKET, SO_RCVTIMEO, (char *)&rcvtimeo, &optlen) < 0)
         {
             rcvtimeo.tv_sec = -1;
         }
@@ -1153,7 +1155,7 @@ static DWORD try_connect(NetSocket *sock,
 #ifdef MPEXT
     if ((timeout > 0) && (rcvtimeo.tv_sec >= 0))
     {
-        p_setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, (void *) &rcvtimeo, sizeof(rcvtimeo));
+        p_setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, (void *) &rcvtimeo, optlen);
     }
 
     // MP: Calling EventSelect only after connect makes sure we receive FD_CLOSE.