* Cast between closure types with a functional cast, not reinterpret_cast
A Borland __closure is a built-in (code, data) pair, so reinterpret_cast
between two closure types is allowed. Under standard C++ a closure is a
class, and reinterpret_cast cannot convert one class object into another.
The functional cast is the old C-style cast this line had before
"Refactoring old-style casts" (e5efed895), so C++Builder compiles it as it
did then, and a compiler implementing closures as classes can give it the
same meaning.
* Read the knocked FTP port's IPv4 address as 32 bits
The address is read through an unsigned long pointer. That is 32 bits on
Windows (LLP64), so it reads exactly the four address bytes there; on LP64
targets unsigned long is 64 bits, and the read runs four bytes past the
address into whatever follows it in gethostbyname's buffer. The assignment
to the 32-bit s_addr then keeps the right bytes on little-endian machines,
which is why it goes unnoticed, but the over-read is undefined. ULONG is 32
bits on every Windows compiler, as Windows API layers for POSIX define it
too.
* Compile the synchronize checklist's Explorer file operation on Windows only
TSynchronizeChecklistFileOperation carries out a synchronization checklist
through Explorer's IFileOperation (a Shell COM object), so that local
deletions go to the recycle bin with Explorer's progress and undo. Only
WinSCP's Windows GUI uses it (CustomScpExplorer.cpp). Elsewhere neither the
object nor the interfaces exist, and declaring them only to compile would
pretend to a facility that is not there; a frontend on another platform
implements the operation its own way. The class and its implementation are
guarded with _WIN32, which C++Builder defines, so the Windows build is
unchanged.
* Declare the SIOCATMARK result as ULONG, the type p_ioctlsocket takes
network.c declares ioctlsocket as taking ULONG * (its DECL_WINDOWS_FUNCTION)
but passes a u_long. Both are 32 bits on Windows, so the two agree there. On
an LP64 platform u_long is 64 bits and ULONG 32, so the pointer types differ
and the callee would write half the variable. ULONG is what the declaration
says, and is the same type on Windows.
* Pick aes-ni.c's _mm_setr_epi8 for any GCC-compatible compiler
MM_SETR_EPI8 uses _mm_setr_epi8 only under _WIN64, falling back to a brace
initialiser that is correct for MSVC's __m128i (a byte union) but fills
clang's and GCC's (a two-lane vector) wrongly; the _WIN64 condition fixed
that for Win64 builds only. Any compiler defining __GNUC__ (clang does) now
takes the intrinsic.
* Compile WinSCP's Windows-only additions to neon on Windows only
WinSCP builds neon for Win32 alone, and three of its additions assume it:
ne_openssl.c includes <windows.h> (it uses nothing from it), timed_connect
switches the socket to non-blocking mode with Winsock's ioctlsocket instead
of upstream's fcntl, and ne_sock_sspi_init calls into SSPI unconditionally.
On POSIX neon uses BSD sockets directly, so the include is guarded with
_WIN32, the two ioctlsocket branches are taken only on _WIN32 (upstream's
fcntl code beside them is used otherwise), and ne_sock_sspi_init calls SSPI
under HAVE_SSPI, as ne_sock_init's own call does, reporting failure (-1)
without it -- which WebDAVFileSystem.cpp already handles as SSPI being
unavailable. C++Builder defines _WIN32 and neon's Win32 config.h defines
HAVE_SSPI, so the Windows build is unchanged.
* Use in_addr's s_addr in FileZilla's proxy layer
AsyncProxySocketLayer.cpp reaches the IPv4 address through Winsock's union,
sin_addr.S_un.S_addr. Winsock also defines s_addr as exactly that member
(#define s_addr S_un.S_addr), and s_addr is the name BSD sockets and POSIX
give the field; a POSIX in_addr has no S_un. Same field, so the Windows
build is unchanged.
* Pass FileZilla's two tool strings as UnicodeString, not std::wstring
CFileZillaTools::GetClientString and CustomReason return std::wstring; the
rest of the interface between core and FileZilla, and both sides' own code,
use UnicodeString or wchar_t pointers. Built with a 2-byte wchar_t on a
platform whose C++ library assumes 4 (clang's -fshort-wchar on Linux),
std::wstring is unusable: libstdc++ provides basic_string<wchar_t> as a
precompiled extern template for its own wchar_t, and char_traits<wchar_t>
calls the C library's 4-byte wcslen and wmemcpy: a Linux build of
FtpFileSystem.cpp imported wcslen through it. The values are only built from
UnicodeString, compared with empty() and read with c_str(), so UnicodeString
is a direct replacement (IsEmpty for empty). On Windows, where wchar_t is 2
bytes throughout, behaviour is unchanged.
* Build storage key paths with backslashes on every platform
THierarchicalStorage joins registry and INI key paths (Sessions\My site)
through its IncludeTrailingBackslash and ExcludeTrailingBackslash members,
which defer to the RTL functions of those names. The RTL's add and strip the
platform's path delimiter: a backslash on Windows, a slash on POSIX, where
these functions also build local file paths. Keys are not file paths: their
separator is a backslash everywhere, and the INI file's sections say so. The
members now add and strip a backslash themselves; on Windows that is what
the RTL did, so the behaviour there is unchanged. Without this, on POSIX the
current key became "Sessions/", matched no section, and no stored site
loaded.
* Hand neon a copy of the TLS-init closure, not its two pointers
SetNeonTlsInit stores the closure in the neon session by reinterpreting it
as a TMethod and saving its code and data pointers, and ne_init_ssl_session
rebuilds it with MakeMethod. That relies on a closure being exactly those
two pointers, which only Borland's __closure is; with any other closure type
the saved words are its internals, read from a by-value parameter that is
gone by the time neon calls back, and the rebuilt closure is garbage (a
clang build's first WebDAV-over-TLS connection crashed there). The session
now holds a heap copy of the closure, freed by neon's destroy-session hook
-- which also covers the sessions libs3 creates and destroys itself -- and
ne_init_ssl_session calls that copy. On C++Builder the behaviour is the
same, at the cost of one small allocation per session.
* Take CString's search results as const wchar_t *
CString::Find, ReverseFind and FindOneOf search the const string c_str()
returns and keep the result in a wchar_t *. ISO C++ declares wcschr,
wcsrchr, wcsstr and wcspbrk on a const string as returning const wchar_t *,
as MSVC's and newer mingw-w64 headers (llvm-mingw's) do, so the
initialisation does not compile there. The pointers are only compared and
subtracted: const is all they need.
* Index the string a string-list property returns, not the property
GetLocales takes a character of Exts->Strings[Index]. Under clang the
__declspec(property) standing for TStrings::Strings' __property reads a
second subscript as the property's own second index, so the expression is a
two-argument call to the getter and does not compile (parentheses do not
change that). Read into a UnicodeString first, the second subscript indexes
the string the property returns, as under C++Builder.
---------
Co-authored-by: Eduardo Correia <ecorreia@apliant.com.br>
TTerminal::LogEvent is defined `inline` in Terminal.cpp, but the file
systems (FtpFileSystem.cpp and others) call it too. An inline function must
be defined in every translation unit that uses it; when it is not, the
program is ill-formed, and whether it links depends on the compiler keeping
an out-of-line copy. An optimizing clang build (-O2, -O3) inlines every call
within Terminal.cpp and emits none, so the other files' calls are undefined
at link time. Without `inline` the definition is the one every caller links
to; the calls within Terminal.cpp may still be inlined.
In the past the predecessor of HandleException probably showed the error, but it did not already in 3.0
Source commit: 9ec53d6edfaef2acf288b0ebbf6f9eb90a242ba4
- regarding the ILINK_LibraryPath for Cfg_1_Win32 (Debug):
Win32\Debug (not only Win32c\Debug) needs to be before Win32c\Release, as win32c does not include debug builds of Pascal code - so without that VCL debugging won't work
- IDE insist on adding CC_Suffix to all projects once switched to Clang, although it is only used in WinSCP.cbproj
- UNICODE define: Not needed for build (it's implicit), but CodeInsight does not know it it somehow
Source commit: da06d73eb86caff5ca9cac0185bed54da0da8f07
Jcl captures exception stack trace on its own, so it's redundant to do it again. And mainly, capturing it the second time somehow does not work with Clang
Source commit: 92a3533d082138b3e5c3729ac35f5fbf0b09b57f
- Once doing that, including more headers to PCH, not just vcl.h
- Clang won't need explicit include, but we will probably keep it anyway.
- #pragma hdrstop has no effect in Clang
Source commit: adc8a268b6907c5337ccaa430cca7526793a3019
Mostly:
– incorrectly identified uninitialized variables
– explicit static global variables
– const, sign and down casts and safety
– typename use
– semicolon after function implementation
– unused variables, fields, arguments and macros
– field initialization order
– exact include filename case
– redundant in code macros already defined at project level
(cherry picked from commit f7f00c631e)
Source commit: 90ad9390853aba308abe73cf02e0281238c53c87
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
(cherry picked from commit 4019b56ffb)
Source commit: 8f8d10bc0c9f4ca626dceda1d3e9889dc22f4394
Overall, the test for cancel is probably redundant for all scenarios. But to limit impact, I'm keeping it for most scenarios, except for the delete.
Source commit: c24c339bd5c5621a74c356b8189487e35dc2ab41
