mirror
/
transmission
mirror of https://github.com/transmission/transmission
1
0
Fork 0
Code Issues Releases Wiki Activity

refactor: cppcoreguidelines-avoid-goto (#3841) 

* refactor: do not use goto in file-win32.cc

* refactor: do not use goto in subprocess-posix.cc

* refactor: do not use goto in peer-io.cc

* build: add cppcoreguidelines-avoid-goto to libtransmission/.clang-tidy
This commit is contained in:
Charles Kerr 2022-09-23 08:39:13 -05:00 committed by GitHub
parent 56e0a1bda8
commit 326d9f3daf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 118 additions and 168 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
libtransmission/.clang-tidy
View File

@ -10,6 +10,7 @@ Checks: >
-cert-err58-cpp,
clang-analyzer-*,
cppcoreguidelines-avoid-const-or-ref-data-members,
cppcoreguidelines-avoid-goto,
cppcoreguidelines-init-variables,
cppcoreguidelines-interfaces-global-init,
cppcoreguidelines-no-malloc,

79
libtransmission/file-win32.cc
View File

@ -8,6 +8,7 @@
#include <cctype> // for isalpha()
#include <cstring>
#include <iterator> // for std::back_inserter
#include <optional>
#include <string>
#include <string_view>
@ -454,58 +455,54 @@ bool tr_sys_path_is_relative(std::string_view path)
return true;
}
static std::optional<BY_HANDLE_FILE_INFORMATION> get_file_info(char const* path, tr_error** error)
{
auto const wpath = path_to_native_path(path);
if (std::empty(wpath))
{
set_system_error_if_file_found(error, GetLastError());
return {};
}
auto const handle = CreateFileW(wpath.c_str(), 0, 0, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
if (handle == INVALID_HANDLE_VALUE)
{
set_system_error_if_file_found(error, GetLastError());
return {};
}
// TODO: Use GetFileInformationByHandleEx on >= Server 2012
auto info = BY_HANDLE_FILE_INFORMATION{};
if (!GetFileInformationByHandle(handle, &info))
{
set_system_error_if_file_found(error, GetLastError());
CloseHandle(handle);
return {};
}
CloseHandle(handle);
return info;
}
bool tr_sys_path_is_same(char const* path1, char const* path2, tr_error** error)
{
TR_ASSERT(path1 != nullptr);
TR_ASSERT(path2 != nullptr);
bool ret = false;
HANDLE handle1 = INVALID_HANDLE_VALUE;
HANDLE handle2 = INVALID_HANDLE_VALUE;
BY_HANDLE_FILE_INFORMATION fi1, fi2;
auto const wide_path1 = path_to_native_path(path1);
auto const wide_path2 = path_to_native_path(path2);
if (std::empty(wide_path1) || std::empty(wide_path2))
auto const fi1 = get_file_info(path1, error);
if (!fi1)
{
goto fail;
return false;
}
handle1 = CreateFileW(wide_path1.c_str(), 0, 0, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
if (handle1 == INVALID_HANDLE_VALUE)
auto const fi2 = get_file_info(path2, error);
if (!fi2)
{
goto fail;
return false;
}
handle2 = CreateFileW(wide_path2.c_str(), 0, 0, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
if (handle2 == INVALID_HANDLE_VALUE)
{
goto fail;
}
/* TODO: Use GetFileInformationByHandleEx on >= Server 2012 */
if (!GetFileInformationByHandle(handle1, &fi1) || !GetFileInformationByHandle(handle2, &fi2))
{
goto fail;
}
ret = fi1.dwVolumeSerialNumber == fi2.dwVolumeSerialNumber && fi1.nFileIndexHigh == fi2.nFileIndexHigh &&
fi1.nFileIndexLow == fi2.nFileIndexLow;
goto cleanup;
fail:
set_system_error_if_file_found(error, GetLastError());
cleanup:
CloseHandle(handle2);
CloseHandle(handle1);
return ret;
return fi1->dwVolumeSerialNumber == fi2->dwVolumeSerialNumber && fi1->nFileIndexHigh == fi2->nFileIndexHigh &&
fi1->nFileIndexLow == fi2->nFileIndexLow;
}
std::string tr_sys_path_resolve(std::string_view path, tr_error** error)

79
libtransmission/peer-io.cc
View File

@ -261,73 +261,48 @@ static void event_write_cb(evutil_socket_t fd, short /*event*/, void* vio)
TR_ASSERT(tr_isPeerIo(io));
TR_ASSERT(io->socket.type == TR_PEER_SOCKET_TYPE_TCP);
auto const dir = TR_UP;
auto res = int{ 0 };
auto what = short{ BEV_EVENT_WRITING };
io->pendingEvents &= ~EV_WRITE;
tr_logAddTraceIo(io, "libevent says this peer is ready to write");
/* Write as much as possible, since the socket is non-blocking, write() will
* return if it can't write any more data without blocking */
size_t const howmuch = io->bandwidth().clamp(dir, evbuffer_get_length(io->outbuf.get()));
// Write as much as possible. Since the socket is non-blocking, write()
// will return if it can't write any more data without blocking.
auto constexpr Dir = TR_UP;
size_t const howmuch = io->bandwidth().clamp(Dir, evbuffer_get_length(io->outbuf.get()));
/* if we don't have any bandwidth left, stop writing */
// if we don't have any bandwidth left, stop writing
if (howmuch < 1)
{
io->setEnabled(dir, false);
io->setEnabled(Dir, false);
return;
}
EVUTIL_SET_SOCKET_ERROR(0);
res = tr_evbuffer_write(io, fd, howmuch);
int const e = EVUTIL_SOCKET_ERROR();
auto const n_written = tr_evbuffer_write(io, fd, howmuch); // -1 on err, 0 on EOF
auto const err = EVUTIL_SOCKET_ERROR();
auto const should_retry = n_written == -1 && (err == 0 || err == EAGAIN || err == EINTR || err == EINPROGRESS);
if (res == -1)
// schedule another write if we have more data to write & think future writes would succeed
if ((evbuffer_get_length(io->outbuf.get()) != 0) && (n_written > 0 || should_retry))
{
if (e == 0 || e == EAGAIN || e == EINTR || e == EINPROGRESS)
io->setEnabled(Dir, true);
}
if (n_written > 0)
{
didWriteWrapper(io, n_written);
}
else
{
auto const what = n_written == -1 ? BEV_EVENT_WRITING | BEV_EVENT_ERROR : BEV_EVENT_WRITING | BEV_EVENT_EOF;
auto const errmsg = tr_net_strerror(err);
tr_logAddDebugIo(
io,
fmt::format("event_write_cb got an err. n_written:{}, what:{}, errno:{} ({})", n_written, what, err, errmsg));
if (io->gotError != nullptr)
{
goto RESCHEDULE;
io->gotError(io, what, io->userData);
}
/* error case */
what |= BEV_EVENT_ERROR;
}
else if (res == 0)
{
/* eof case */
what |= BEV_EVENT_EOF;
}
if (res <= 0)
{
goto FAIL;
}
if (evbuffer_get_length(io->outbuf.get()) != 0)
{
io->setEnabled(dir, true);
}
didWriteWrapper(io, res);
return;
RESCHEDULE:
if (evbuffer_get_length(io->outbuf.get()) != 0)
{
io->setEnabled(dir, true);
}
return;
FAIL:
auto const errmsg = tr_net_strerror(e);
tr_logAddDebugIo(io, fmt::format("event_write_cb got an err. res:{}, what:{}, errno:{} ({})", res, what, e, errmsg));
if (io->gotError != nullptr)
{
io->gotError(io, what, io->userData);
}
}

16
libtransmission/subprocess-posix.cc
View File

@ -53,8 +53,7 @@ static void set_system_error(tr_error** error, int code, std::string_view what)
static bool tr_spawn_async_in_child(
char const* const* cmd,
std::map<std::string_view, std::string_view> const& env,
std::string_view work_dir,
int pipe_fd)
std::string_view work_dir)
{
auto key_sz = std::string{};
auto val_sz = std::string{};
@ -66,25 +65,21 @@ static bool tr_spawn_async_in_child(
if (setenv(key_sz.c_str(), val_sz.c_str(), 1) != 0)
{
goto FAIL;
return false;
}
}
if (!std::empty(work_dir) && chdir(tr_pathbuf{ work_dir }) == -1)
{
goto FAIL;
return false;
}
if (execvp(cmd[0], const_cast<char* const*>(cmd)) == -1)
{
goto FAIL;
return false;
}
return true;
FAIL:
(void)write(pipe_fd, &errno, sizeof(errno));
return false;
}
static bool tr_spawn_async_in_parent(int pipe_fd, tr_error** error)
@ -168,8 +163,9 @@ bool tr_spawn_async(
{
close(pipe_fds[0]);
if (!tr_spawn_async_in_child(cmd, env, work_dir, pipe_fds[1]))
if (!tr_spawn_async_in_child(cmd, env, work_dir))
{
(void)write(pipe_fds[1], &errno, sizeof(errno));
_exit(0);
}
}

111
libtransmission/tr-udp.cc
View File

@ -104,15 +104,39 @@ void tr_session::tr_udp_core::set_socket_buffers()
}
}
static tr_socket_t rebind_ipv6_impl(in6_addr sin6_addr, tr_port port)
{
auto const sock = socket(PF_INET6, SOCK_DGRAM, 0);
if (sock == TR_BAD_SOCKET)
{
return TR_BAD_SOCKET;
}
#ifdef IPV6_V6ONLY
/* Since we always open an IPv4 socket on the same port,
* this shouldn't matter. But I'm superstitious. */
int one = 1;
(void)setsockopt(sock, IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast<char const*>(&one), sizeof(one));
#endif
auto sin6 = sockaddr_in6{};
sin6.sin6_family = AF_INET6;
sin6.sin6_addr = sin6_addr;
sin6.sin6_port = port.network();
if (::bind(sock, reinterpret_cast<struct sockaddr*>(&sin6), sizeof(sin6)) == -1)
{
tr_netCloseSocket(sock);
return TR_BAD_SOCKET;
}
return sock;
}
/* BEP-32 has a rather nice explanation of why we need to bind to one
IPv6 address, if I may say so myself. */
// TODO: remove goto, it prevents reducing scope of local variables
void tr_session::tr_udp_core::rebind_ipv6(bool force)
{
auto sin6 = sockaddr_in6{};
auto const ipv6 = tr_globalIPv6(&session_);
int rc = -1;
int one = 1;
/* We currently have no way to enable or disable IPv6 after initialisation.
No way to fix that without some surgery to the DHT code itself. */
@ -127,74 +151,31 @@ void tr_session::tr_udp_core::rebind_ipv6(bool force)
return;
}
auto const s = socket(PF_INET6, SOCK_DGRAM, 0);
if (s == TR_BAD_SOCKET)
auto const sock = rebind_ipv6_impl(*ipv6, udp_port_);
if (sock == TR_BAD_SOCKET)
{
goto FAIL;
/* Something went wrong. It's difficult to recover, so let's
* simply set things up so that we try again next time. */
auto const error_code = errno;
auto ipv6_readable = std::array<char, INET6_ADDRSTRLEN>{};
evutil_inet_ntop(AF_INET6, &*ipv6, std::data(ipv6_readable), std::size(ipv6_readable));
tr_logAddWarn(fmt::format(
_("Couldn't rebind IPv6 socket {address}: {error} ({error_code})"),
fmt::arg("address", std::data(ipv6_readable)),
fmt::arg("error", tr_strerror(error_code)),
fmt::arg("error_code", error_code)));
udp6_bound_.reset();
return;
}
#ifdef IPV6_V6ONLY
/* Since we always open an IPv4 socket on the same port, this
shouldn't matter. But I'm superstitious. */
(void)setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast<char const*>(&one), sizeof(one));
#endif
memset(&sin6, 0, sizeof(sin6));
sin6.sin6_family = AF_INET6;
if (ipv6)
if (udp6_socket_ != TR_BAD_SOCKET)
{
sin6.sin6_addr = *ipv6;
}
sin6.sin6_port = udp_port_.network();
rc = bind(s, (struct sockaddr*)&sin6, sizeof(sin6));
if (rc == -1)
{
goto FAIL;
}
if (udp6_socket_ == TR_BAD_SOCKET)
{
udp6_socket_ = s;
}
else
{
/* FIXME: dup2 doesn't work for sockets on Windows */
rc = dup2(s, udp6_socket_);
if (rc == -1)
{
goto FAIL;
}
tr_netCloseSocket(s);
tr_netCloseSocket(udp6_socket_);
}
udp6_socket_ = sock;
udp6_bound_ = ipv6;
return;
FAIL:
/* Something went wrong. It's difficult to recover, so let's simply
set things up so that we try again next time. */
auto const error_code = errno;
auto ipv6_readable = std::array<char, INET6_ADDRSTRLEN>{};
evutil_inet_ntop(AF_INET6, &*ipv6, std::data(ipv6_readable), std::size(ipv6_readable));
tr_logAddWarn(fmt::format(
_("Couldn't rebind IPv6 socket {address}: {error} ({error_code})"),
fmt::arg("address", std::data(ipv6_readable)),
fmt::arg("error", tr_strerror(error_code)),
fmt::arg("error_code", error_code)));
if (s != TR_BAD_SOCKET)
{
tr_netCloseSocket(s);
}
udp6_bound_.reset();
}
static void event_callback(evutil_socket_t s, [[maybe_unused]] short type, void* vsession)