72cae4996f
fix off-by-one |
||
---|---|---|
.. | ||
.clang-tidy | ||
CMakeLists.txt | ||
ConvertUTF.c | ||
ConvertUTF.h | ||
README.md | ||
announcer-common.h | ||
announcer-http.cc | ||
announcer-udp.cc | ||
announcer.cc | ||
announcer.h | ||
bandwidth.cc | ||
bandwidth.h | ||
bitfield.cc | ||
bitfield.h | ||
block-info.cc | ||
block-info.h | ||
blocklist.cc | ||
blocklist.h | ||
cache.cc | ||
cache.h | ||
clients.cc | ||
clients.h | ||
completion.cc | ||
completion.h | ||
crypto-utils-ccrypto.cc | ||
crypto-utils-cyassl.cc | ||
crypto-utils-fallback.cc | ||
crypto-utils-openssl.cc | ||
crypto-utils-polarssl.cc | ||
crypto-utils.cc | ||
crypto-utils.h | ||
crypto.cc | ||
crypto.h | ||
error-types.h | ||
error.cc | ||
error.h | ||
fdlimit.cc | ||
fdlimit.h | ||
file-posix.cc | ||
file-win32.cc | ||
file.cc | ||
file.h | ||
handshake.cc | ||
handshake.h | ||
history.h | ||
inout.cc | ||
inout.h | ||
jsonsl.c | ||
jsonsl.h | ||
libt.dox | ||
log.cc | ||
log.h | ||
magnet-metainfo.cc | ||
magnet-metainfo.h | ||
makemeta.cc | ||
makemeta.h | ||
metainfo.cc | ||
metainfo.h | ||
mime-types.h | ||
mime-types.js | ||
natpmp.cc | ||
natpmp_local.h | ||
net.cc | ||
net.h | ||
peer-common.h | ||
peer-io.cc | ||
peer-io.h | ||
peer-mgr-active-requests.cc | ||
peer-mgr-active-requests.h | ||
peer-mgr-wishlist.cc | ||
peer-mgr-wishlist.h | ||
peer-mgr.cc | ||
peer-mgr.h | ||
peer-msgs.cc | ||
peer-msgs.h | ||
peer-socket.h | ||
platform-quota.cc | ||
platform-quota.h | ||
platform.cc | ||
platform.h | ||
port-forwarding.cc | ||
port-forwarding.h | ||
ptrarray.cc | ||
ptrarray.h | ||
quark.cc | ||
quark.h | ||
resume.cc | ||
resume.h | ||
rpc-server.cc | ||
rpc-server.h | ||
rpcimpl.cc | ||
rpcimpl.h | ||
session-id.cc | ||
session-id.h | ||
session.cc | ||
session.h | ||
stats.cc | ||
stats.h | ||
subprocess-posix.cc | ||
subprocess-win32.cc | ||
subprocess.h | ||
torrent-ctor.cc | ||
torrent-magnet.cc | ||
torrent-magnet.h | ||
torrent.cc | ||
torrent.h | ||
tr-assert.cc | ||
tr-assert.h | ||
tr-dht.cc | ||
tr-dht.h | ||
tr-getopt.cc | ||
tr-getopt.h | ||
tr-lpd.cc | ||
tr-lpd.h | ||
tr-macros.h | ||
tr-udp.cc | ||
tr-udp.h | ||
tr-utp.cc | ||
tr-utp.h | ||
transmission.h | ||
trevent.cc | ||
trevent.h | ||
upnp.cc | ||
upnp.h | ||
utils.cc | ||
utils.h | ||
variant-benc.cc | ||
variant-common.h | ||
variant-json.cc | ||
variant.cc | ||
variant.h | ||
verify.cc | ||
verify.h | ||
version.h.in | ||
watchdir-common.h | ||
watchdir-generic.cc | ||
watchdir-inotify.cc | ||
watchdir-kqueue.cc | ||
watchdir-win32.cc | ||
watchdir.cc | ||
watchdir.h | ||
web-utils.cc | ||
web-utils.h | ||
web.cc | ||
web.h | ||
webseed.cc | ||
webseed.h | ||
wildmat.c |
README.md
Notes on the C-to-C++ Conversion
-
libtransmission was written in C for fifteen years, so eliminating all Cisms is nearly impossible. Modernization patches are welcomed but it won't all happen overnight.
tr_strdup()
andconstexpr
wil exist side-by-side in the codebase for the forseeable future. -
It's so tempting to refactor all the things! Please keep modernization patches reasonably focused so that they will be easy to review.
-
Prefer
std::
tools over bespoke ones. For example, usestd::vector
instead of tr_ptrArray. Redundant bespoke code should be removed. -
Consider ripple effects before adding C++ into public headers. Will it break C code that #includes that header? If you think it might, consult with downstream projects to see if this is a problem for them.
Checklist for modernization of a module
NOTE:
The version inlibtransmission/CMakeLists.txt
is C++17.
See https://github.com/AnthonyCalandra/modern-cpp-features
This can be done in multiple smaller passes:
- Satisfy clang-tidy.
libtransmission/.clang-tidy
's list of rules should eventually be expanded to a list similar to the one inqt/.clang-tidy
. - Group member functions and their parent structs
- Use
namespace libtransmission
- Split large modules into smaller groups of files in a
libtransmission/<name>
subdirectories, with own sub-namespace.
- Use
- Enums replaced with new
enum class
syntax. Numeric#define
constants replaced with C++const
/constexpr
. - Memory - promote init and free functions to C++ ctors and dtors, and ensure it is only managed with new/delete
- Owned memory - promote simple pointer fields owning their data to smart pointers (unique_ptr, shared_ptr, vector, string)
Detailed Steps
-
Satisfy clang-tidy warnings
- Change C includes to C++ wraps, example: <string.h> becomes and update calls to standard library to
use
std::
namespace prefix. This clearly delineates the border between std library and transmission. Headers must be sorted alphabetically. - Headers which are used conditionally based on some
#ifdef
in the code, should also have same#ifdef
in the include section. - Revisit type warnings,
int
vsunsigned int
. Sizes and counts should usesize_t
type where this does not break external API declarations. Ideally change that too.
- Change C includes to C++ wraps, example: <string.h> becomes and update calls to standard library to
use
-
Move and group code together.
- Move member functions into structs. To minimize code churn, create function forward declarations inside structs,
and give
struct_name::
prefixes to the functions.
becomes:typedef struct { int field; } foo; int foo_blep(struct foo *f) { return f->field; }
struct foo { int field; void blep(); }; int foo::blep() { return this->field; }
- For functions taking
const
pointer, addconst
after the function prototype:int blep() const
like so. - For structs used by other modules, struct definitions should relocate to internal
*-common.h
header files. - Split large files into sub-modules sharing own separate sub-namespace and sitting in a subdirectory
under
libtransmission/
. - Some externally invoked functions must either not move OR have
extern "C"
adapter functions.
- Move member functions into structs. To minimize code churn, create function forward declarations inside structs,
and give
-
Enums promoted to
enum class
and given a type:enum { A, B, C };
becomes
enum: int { A, B, C }; // unscoped, use A, B, C enum MyEnum: int { A, B, C }; // unscoped, use A, B, C // OR wrap into a scope - enum struct MyEnum: int { A, B, C }; // scoped, use MyEnum::A enum class MyEnum: int { A, B, C }; // scoped, use MyEnum::A
this will make all values of enum to have that numeric type.
-
Numeric/bool
#define
constants should be replaced with C++const
/constexpr
. -
Memory management:
- Prefer constructors and destructors vs manual construction and destruction. But when doing so must ensure that the
struct is never constructed using C
malloc
/free
, but must use C++new
/delete
. - Avoid using std::memset on a new struct. It is allowed in C, and C++ struct but will destroy virtual table on a C++ class. Use field initializers in C++.
- Prefer constructors and destructors vs manual construction and destruction. But when doing so must ensure that the
struct is never constructed using C
-
Owned memory:
- If destructor deletes something, it means it was owned. Promote that field to owning type (vector, unique_ptr, shared_ptr or string).