* 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>
with aim to eventually update the code to need as little warnings disabled as needed.
Some code updates (and even fixes) to avoid warning done already
Source commit: 5c45a6f56c52e7b0c5034ed9980cd65259a7372b
- 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
Source commit: 5c1338d95bd275082288dd1e773a033df6e379cd
