From 93c83fbc7caf683014a19454d6ab3b703e65e9d7 Mon Sep 17 00:00:00 2001 From: iphydf Date: Sat, 3 Feb 2024 21:30:51 +0000 Subject: [PATCH 1/2] refactor: Use strong typedef instead of struct for `Socket`. Sparse checks it. This is neater than using a struct, which has some slightly weird syntax at times. This also reduces the risk of someone adding another struct member. --- .clang-tidy | 2 +- .github/workflows/ci.yml | 3 +- other/docker/sparse/.gitignore | 1 + other/docker/sparse/Makefile | 70 +++++++++++++++++ other/docker/sparse/local.mk | 1 + other/docker/sparse/run | 6 ++ other/docker/sparse/sparse.Dockerfile | 35 +++++++++ testing/fuzzing/fuzz_support.cc | 97 +++++++++++------------ toxcore/LAN_discovery.c | 4 +- toxcore/TCP_server.c | 18 ++--- toxcore/attributes.h | 8 ++ toxcore/network.c | 106 ++++++++++++++------------ toxcore/network.h | 38 ++++----- 13 files changed, 262 insertions(+), 127 deletions(-) create mode 100644 other/docker/sparse/.gitignore create mode 100644 other/docker/sparse/Makefile create mode 100644 other/docker/sparse/local.mk create mode 100755 other/docker/sparse/run create mode 100644 other/docker/sparse/sparse.Dockerfile diff --git a/.clang-tidy b/.clang-tidy index 6f0fd59fed..d41d6628bf 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -21,7 +21,7 @@ CheckOptions: - key: readability-identifier-naming.MacroDefinitionCase value: UPPER_CASE - key: readability-identifier-naming.MacroDefinitionIgnoredRegexp - value: "^_.*|nullable|non_null|nullptr|static_assert|ck_.*" + value: "^_.*|bitwise|force|nullable|non_null|nullptr|static_assert|ck_.*" - key: readability-identifier-naming.ParameterCase value: lower_case - key: readability-identifier-naming.StructCase diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b05e6d59fa..b24fb9d615 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,8 +15,9 @@ jobs: analysis: strategy: + fail-fast: false matrix: - tool: [autotools, clang-tidy, compcert, cppcheck, doxygen, goblint, infer, misra, modules, rpm, slimcc, tcc, tokstyle] + tool: [autotools, clang-tidy, compcert, cppcheck, doxygen, goblint, infer, misra, modules, rpm, slimcc, sparse, tcc, tokstyle] runs-on: ubuntu-latest steps: - name: Set up Docker Buildx diff --git a/other/docker/sparse/.gitignore b/other/docker/sparse/.gitignore new file mode 100644 index 0000000000..2526423199 --- /dev/null +++ b/other/docker/sparse/.gitignore @@ -0,0 +1 @@ +!/Makefile diff --git a/other/docker/sparse/Makefile b/other/docker/sparse/Makefile new file mode 100644 index 0000000000..ce567b7296 --- /dev/null +++ b/other/docker/sparse/Makefile @@ -0,0 +1,70 @@ +SOURCES := $(wildcard tox*/*.c tox*/*/*.c) \ + third_party/cmp/cmp.c +OBJECTS := $(SOURCES:.c=.o) + +CFLAGS := $(shell pkg-config --cflags libsodium opus vpx) +CPPFLAGS := -DSPARSE -DTCP_SERVER_USE_EPOLL=1 -DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE + +SPARSE_FLAGS := \ + -Wsparse-error \ + -Wpedantic \ + -Waddress \ + -Waddress-space \ + -Wbitwise \ + -Wbitwise-pointer \ + -Wcast-from-as \ + -Wcast-to-as \ + -Wcast-truncate \ + -Wconstant-suffix \ + -Wconstexpr-not-const \ + -Wcontext \ + -Wdecl \ + -Wdefault-bitfield-sign \ + -Wdesignated-init \ + -Wdo-while \ + -Wenum-mismatch \ + -Wexternal-function-has-definition \ + -Wflexible-array-array \ + -Wflexible-array-nested \ + -Wflexible-array-union \ + -Wimplicit-int \ + -Winit-cstring \ + -Wint-to-pointer-cast \ + -Wmemcpy-max-count \ + -Wnon-pointer-null \ + -Wnewline-eof \ + -Wold-initializer \ + -Wold-style-definition \ + -Wone-bit-signed-bitfield \ + -Woverride-init \ + -Woverride-init-all \ + -Wparen-string \ + -Wpast-deep-designator \ + -Wpedantic \ + -Wpointer-to-int-cast \ + -Wptr-subtraction-blows \ + -Wreturn-void \ + -Wshadow \ + -Wshift-count-negative \ + -Wshift-count-overflow \ + -Wsizeof-bool \ + -Wstrict-prototypes \ + -Wpointer-arith \ + -Wsparse-error \ + -Wtautological-compare \ + -Wtransparent-union \ + -Wtypesign \ + -Wundef \ + -Wuninitialized \ + -Wunion-cast \ + -Wvla + +SMATCH_FLAGS := $(foreach i,$(shell smatch --show-checks | grep -o 'check_.*'),--enable=$i) + +analyse: $(OBJECTS) + +%.o: %.c + @echo "Processing $<" + @sparse $(CFLAGS) $(CPPFLAGS) $(SPARSE_FLAGS) $< +# @smatch $(CFLAGS) $(CPPFLAGS) $(SMATCH_FLAGS) $< +# @sparse-llvm $(CFLAGS) $(CPPFLAGS) $< > /dev/null diff --git a/other/docker/sparse/local.mk b/other/docker/sparse/local.mk new file mode 100644 index 0000000000..b67f33b8b0 --- /dev/null +++ b/other/docker/sparse/local.mk @@ -0,0 +1 @@ +CFLAGS=-O3 -g -Wno-discarded-qualifiers -Wno-format-truncation -Wno-stringop-truncation -Wno-uninitialized -Wno-unused -Wno-unused-result diff --git a/other/docker/sparse/run b/other/docker/sparse/run new file mode 100755 index 0000000000..6b87e91b8a --- /dev/null +++ b/other/docker/sparse/run @@ -0,0 +1,6 @@ +#!/bin/sh + +set -eux +BUILD=sparse +other/docker/sources/build +docker build -t "toxchat/c-toxcore:$BUILD" -f "other/docker/$BUILD/$BUILD.Dockerfile" . diff --git a/other/docker/sparse/sparse.Dockerfile b/other/docker/sparse/sparse.Dockerfile new file mode 100644 index 0000000000..e4e185a89e --- /dev/null +++ b/other/docker/sparse/sparse.Dockerfile @@ -0,0 +1,35 @@ +FROM toxchat/c-toxcore:sources AS sources +FROM ubuntu:22.04 + +RUN apt-get update && \ + DEBIAN_FRONTEND="noninteractive" apt-get install -y --no-install-recommends \ + ca-certificates \ + creduce \ + g++ \ + gcc \ + git \ + libc-dev \ + libopus-dev \ + libsodium-dev \ + libsqlite3-dev \ + libssl-dev \ + libvpx-dev \ + llvm-dev \ + make \ + pkg-config \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /work/smatch +RUN git clone --depth=1 https://repo.or.cz/smatch.git /work/smatch +COPY other/docker/sparse/local.mk /work/smatch/local.mk +RUN make install -j4 PREFIX=/usr/local + +WORKDIR /work/c-toxcore +COPY --from=sources /src/ /work/c-toxcore +#COPY other/make_single_file /work/c-toxcore/other/ +#RUN other/make_single_file auto_tests/tox_new_test.c > crash.c +#RUN sparsec $(pkg-config --cflags --libs libsodium opus vpx) crash.c + +COPY other/docker/sparse/Makefile /work/c-toxcore/ +RUN make -j4 diff --git a/testing/fuzzing/fuzz_support.cc b/testing/fuzzing/fuzz_support.cc index e1a39078be..fe760f9958 100644 --- a/testing/fuzzing/fuzz_support.cc +++ b/testing/fuzzing/fuzz_support.cc @@ -107,25 +107,25 @@ static constexpr Memory_Funcs fuzz_memory_funcs = { }; static constexpr Network_Funcs fuzz_network_funcs = { - /* .close = */ ![](Fuzz_System *self, int sock) { return 0; }, - /* .accept = */ ![](Fuzz_System *self, int sock) { return 1337; }, - /* .bind = */ ![](Fuzz_System *self, int sock, const Network_Addr *addr) { return 0; }, - /* .listen = */ ![](Fuzz_System *self, int sock, int backlog) { return 0; }, + /* .close = */ ![](Fuzz_System *self, Socket sock) { return 0; }, + /* .accept = */ ![](Fuzz_System *self, Socket sock) { return Socket{1337}; }, + /* .bind = */ ![](Fuzz_System *self, Socket sock, const Network_Addr *addr) { return 0; }, + /* .listen = */ ![](Fuzz_System *self, Socket sock, int backlog) { return 0; }, /* .recvbuf = */ - ![](Fuzz_System *self, int sock) { - assert(sock == 42 || sock == 1337); + ![](Fuzz_System *self, Socket sock) { + assert(sock.value == 42 || sock.value == 1337); const size_t count = random_u16(self->rng.get()); return static_cast(std::min(count, self->data.size())); }, /* .recv = */ - ![](Fuzz_System *self, int sock, uint8_t *buf, size_t len) { - assert(sock == 42 || sock == 1337); + ![](Fuzz_System *self, Socket sock, uint8_t *buf, size_t len) { + assert(sock.value == 42 || sock.value == 1337); // Receive data from the fuzzer. return recv_common(self->data, buf, len); }, /* .recvfrom = */ - ![](Fuzz_System *self, int sock, uint8_t *buf, size_t len, Network_Addr *addr) { - assert(sock == 42 || sock == 1337); + ![](Fuzz_System *self, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { + assert(sock.value == 42 || sock.value == 1337); addr->addr = sockaddr_storage{}; // Dummy Addr @@ -140,26 +140,26 @@ static constexpr Network_Funcs fuzz_network_funcs = { return recv_common(self->data, buf, len); }, /* .send = */ - ![](Fuzz_System *self, int sock, const uint8_t *buf, size_t len) { - assert(sock == 42 || sock == 1337); + ![](Fuzz_System *self, Socket sock, const uint8_t *buf, size_t len) { + assert(sock.value == 42 || sock.value == 1337); // Always succeed. return static_cast(len); }, /* .sendto = */ - ![](Fuzz_System *self, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { - assert(sock == 42 || sock == 1337); + ![](Fuzz_System *self, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { + assert(sock.value == 42 || sock.value == 1337); // Always succeed. return static_cast(len); }, - /* .socket = */ ![](Fuzz_System *self, int domain, int type, int proto) { return 42; }, - /* .socket_nonblock = */ ![](Fuzz_System *self, int sock, bool nonblock) { return 0; }, + /* .socket = */ ![](Fuzz_System *self, int domain, int type, int proto) { return Socket{42}; }, + /* .socket_nonblock = */ ![](Fuzz_System *self, Socket sock, bool nonblock) { return 0; }, /* .getsockopt = */ - ![](Fuzz_System *self, int sock, int level, int optname, void *optval, size_t *optlen) { + ![](Fuzz_System *self, Socket sock, int level, int optname, void *optval, size_t *optlen) { std::memset(optval, 0, *optlen); return 0; }, /* .setsockopt = */ - ![](Fuzz_System *self, int sock, int level, int optname, const void *optval, size_t optlen) { + ![](Fuzz_System *self, Socket sock, int level, int optname, const void *optval, size_t optlen) { return 0; }, }; @@ -221,42 +221,42 @@ static constexpr Memory_Funcs null_memory_funcs = { }; static constexpr Network_Funcs null_network_funcs = { - /* .close = */ ![](Null_System *self, int sock) { return 0; }, - /* .accept = */ ![](Null_System *self, int sock) { return 1337; }, - /* .bind = */ ![](Null_System *self, int sock, const Network_Addr *addr) { return 0; }, - /* .listen = */ ![](Null_System *self, int sock, int backlog) { return 0; }, - /* .recvbuf = */ ![](Null_System *self, int sock) { return 0; }, + /* .close = */ ![](Null_System *self, Socket sock) { return 0; }, + /* .accept = */ ![](Null_System *self, Socket sock) { return Socket{1337}; }, + /* .bind = */ ![](Null_System *self, Socket sock, const Network_Addr *addr) { return 0; }, + /* .listen = */ ![](Null_System *self, Socket sock, int backlog) { return 0; }, + /* .recvbuf = */ ![](Null_System *self, Socket sock) { return 0; }, /* .recv = */ - ![](Null_System *self, int sock, uint8_t *buf, size_t len) { + ![](Null_System *self, Socket sock, uint8_t *buf, size_t len) { // Always fail. errno = ENOMEM; return -1; }, /* .recvfrom = */ - ![](Null_System *self, int sock, uint8_t *buf, size_t len, Network_Addr *addr) { + ![](Null_System *self, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { // Always fail. errno = ENOMEM; return -1; }, /* .send = */ - ![](Null_System *self, int sock, const uint8_t *buf, size_t len) { + ![](Null_System *self, Socket sock, const uint8_t *buf, size_t len) { // Always succeed. return static_cast(len); }, /* .sendto = */ - ![](Null_System *self, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { + ![](Null_System *self, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { // Always succeed. return static_cast(len); }, - /* .socket = */ ![](Null_System *self, int domain, int type, int proto) { return 42; }, - /* .socket_nonblock = */ ![](Null_System *self, int sock, bool nonblock) { return 0; }, + /* .socket = */ ![](Null_System *self, int domain, int type, int proto) { return Socket{42}; }, + /* .socket_nonblock = */ ![](Null_System *self, Socket sock, bool nonblock) { return 0; }, /* .getsockopt = */ - ![](Null_System *self, int sock, int level, int optname, void *optval, size_t *optlen) { + ![](Null_System *self, Socket sock, int level, int optname, void *optval, size_t *optlen) { std::memset(optval, 0, *optlen); return 0; }, /* .setsockopt = */ - ![](Null_System *self, int sock, int level, int optname, const void *optval, size_t optlen) { + ![](Null_System *self, Socket sock, int level, int optname, const void *optval, size_t optlen) { return 0; }, }; @@ -327,10 +327,10 @@ static constexpr Memory_Funcs record_memory_funcs = { }; static constexpr Network_Funcs record_network_funcs = { - /* .close = */ ![](Record_System *self, int sock) { return 0; }, - /* .accept = */ ![](Record_System *self, int sock) { return 2; }, + /* .close = */ ![](Record_System *self, Socket sock) { return 0; }, + /* .accept = */ ![](Record_System *self, Socket sock) { return Socket{2}; }, /* .bind = */ - ![](Record_System *self, int sock, const Network_Addr *addr) { + ![](Record_System *self, Socket sock, const Network_Addr *addr) { const uint16_t port = get_port(addr); if (self->global_.bound.find(port) != self->global_.bound.end()) { errno = EADDRINUSE; @@ -340,17 +340,17 @@ static constexpr Network_Funcs record_network_funcs = { self->port = port; return 0; }, - /* .listen = */ ![](Record_System *self, int sock, int backlog) { return 0; }, - /* .recvbuf = */ ![](Record_System *self, int sock) { return 0; }, + /* .listen = */ ![](Record_System *self, Socket sock, int backlog) { return 0; }, + /* .recvbuf = */ ![](Record_System *self, Socket sock) { return 0; }, /* .recv = */ - ![](Record_System *self, int sock, uint8_t *buf, size_t len) { + ![](Record_System *self, Socket sock, uint8_t *buf, size_t len) { // Always fail. errno = ENOMEM; return -1; }, /* .recvfrom = */ - ![](Record_System *self, int sock, uint8_t *buf, size_t len, Network_Addr *addr) { - assert(sock == 42); + ![](Record_System *self, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { + assert(sock.value == 42); if (self->recvq.empty()) { self->push("\xff\xff"); errno = EWOULDBLOCK; @@ -385,29 +385,30 @@ static constexpr Network_Funcs record_network_funcs = { return static_cast(recvlen); }, /* .send = */ - ![](Record_System *self, int sock, const uint8_t *buf, size_t len) { + ![](Record_System *self, Socket sock, const uint8_t *buf, size_t len) { // Always succeed. return static_cast(len); }, /* .sendto = */ - ![](Record_System *self, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { - assert(sock == 42); + ![](Record_System *self, Socket sock, const uint8_t *buf, size_t len, + const Network_Addr *addr) { + assert(sock.value == 42); auto backend = self->global_.bound.find(get_port(addr)); assert(backend != self->global_.bound.end()); backend->second->receive(self->port, buf, len); return static_cast(len); }, - /* .socket = */ ![](Record_System *self, int domain, int type, int proto) { return 42; }, - /* .socket_nonblock = */ ![](Record_System *self, int sock, bool nonblock) { return 0; }, + /* .socket = */ + ![](Record_System *self, int domain, int type, int proto) { return Socket{42}; }, + /* .socket_nonblock = */ ![](Record_System *self, Socket sock, bool nonblock) { return 0; }, /* .getsockopt = */ - ![](Record_System *self, int sock, int level, int optname, void *optval, size_t *optlen) { + ![](Record_System *self, Socket sock, int level, int optname, void *optval, size_t *optlen) { std::memset(optval, 0, *optlen); return 0; }, /* .setsockopt = */ - ![](Record_System *self, int sock, int level, int optname, const void *optval, size_t optlen) { - return 0; - }, + ![](Record_System *self, Socket sock, int level, int optname, const void *optval, + size_t optlen) { return 0; }, }; static constexpr Random_Funcs record_random_funcs = { diff --git a/toxcore/LAN_discovery.c b/toxcore/LAN_discovery.c index bf3ec98b15..aead975911 100644 --- a/toxcore/LAN_discovery.c +++ b/toxcore/LAN_discovery.c @@ -148,7 +148,7 @@ static Broadcast_Info *fetch_broadcast_info(const Network *ns) ifc.ifc_buf = (char *)i_faces; ifc.ifc_len = sizeof(i_faces); - if (ioctl(sock.sock, SIOCGIFCONF, &ifc) < 0) { + if (ioctl(net_socket_to_native(sock), SIOCGIFCONF, &ifc) < 0) { kill_sock(ns, sock); free(broadcast); return nullptr; @@ -163,7 +163,7 @@ static Broadcast_Info *fetch_broadcast_info(const Network *ns) for (int i = 0; i < n; ++i) { /* there are interfaces with are incapable of broadcast */ - if (ioctl(sock.sock, SIOCGIFBRDADDR, &i_faces[i]) < 0) { + if (ioctl(net_socket_to_native(sock), SIOCGIFBRDADDR, &i_faces[i]) < 0) { continue; } diff --git a/toxcore/TCP_server.c b/toxcore/TCP_server.c index 3ce72e8e70..1363d90220 100644 --- a/toxcore/TCP_server.c +++ b/toxcore/TCP_server.c @@ -1009,9 +1009,9 @@ TCP_Server *new_tcp_server(const Logger *logger, const Memory *mem, const Random struct epoll_event ev; ev.events = EPOLLIN | EPOLLET; - ev.data.u64 = sock.sock | ((uint64_t)TCP_SOCKET_LISTENING << 32); + ev.data.u64 = net_socket_to_native(sock) | ((uint64_t)TCP_SOCKET_LISTENING << 32); - if (epoll_ctl(temp->efd, EPOLL_CTL_ADD, sock.sock, &ev) == -1) { + if (epoll_ctl(temp->efd, EPOLL_CTL_ADD, net_socket_to_native(sock), &ev) == -1) { continue; } @@ -1248,7 +1248,7 @@ static bool tcp_epoll_process(TCP_Server *tcp_server, const Mono_Time *mono_time #undef MAX_EVENTS for (int n = 0; n < nfds; ++n) { - const Socket sock = {(int)(events[n].data.u64 & 0xFFFFFFFF)}; + const Socket sock = net_socket_from_native((int)(events[n].data.u64 & 0xFFFFFFFF)); const int status = (events[n].data.u64 >> 32) & 0xFF; const int index = events[n].data.u64 >> 40; @@ -1306,9 +1306,9 @@ static bool tcp_epoll_process(TCP_Server *tcp_server, const Mono_Time *mono_time ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP; - ev.data.u64 = sock_new.sock | ((uint64_t)TCP_SOCKET_INCOMING << 32) | ((uint64_t)index_new << 40); + ev.data.u64 = net_socket_to_native(sock_new) | ((uint64_t)TCP_SOCKET_INCOMING << 32) | ((uint64_t)index_new << 40); - if (epoll_ctl(tcp_server->efd, EPOLL_CTL_ADD, sock_new.sock, &ev) == -1) { + if (epoll_ctl(tcp_server->efd, EPOLL_CTL_ADD, net_socket_to_native(sock_new), &ev) == -1) { LOGGER_DEBUG(tcp_server->logger, "new connection %d was dropped due to epoll error %d", index, net_error()); kill_tcp_secure_connection(&tcp_server->incoming_connection_queue[index_new]); continue; @@ -1324,9 +1324,9 @@ static bool tcp_epoll_process(TCP_Server *tcp_server, const Mono_Time *mono_time if (index_new != -1) { LOGGER_TRACE(tcp_server->logger, "incoming connection %d was accepted as %d", index, index_new); events[n].events = EPOLLIN | EPOLLET | EPOLLRDHUP; - events[n].data.u64 = sock.sock | ((uint64_t)TCP_SOCKET_UNCONFIRMED << 32) | ((uint64_t)index_new << 40); + events[n].data.u64 = net_socket_to_native(sock) | ((uint64_t)TCP_SOCKET_UNCONFIRMED << 32) | ((uint64_t)index_new << 40); - if (epoll_ctl(tcp_server->efd, EPOLL_CTL_MOD, sock.sock, &events[n]) == -1) { + if (epoll_ctl(tcp_server->efd, EPOLL_CTL_MOD, net_socket_to_native(sock), &events[n]) == -1) { LOGGER_DEBUG(tcp_server->logger, "incoming connection %d was dropped due to epoll error %d", index, net_error()); kill_tcp_secure_connection(&tcp_server->unconfirmed_connection_queue[index_new]); break; @@ -1342,9 +1342,9 @@ static bool tcp_epoll_process(TCP_Server *tcp_server, const Mono_Time *mono_time if (index_new != -1) { LOGGER_TRACE(tcp_server->logger, "unconfirmed connection %d was confirmed as %d", index, index_new); events[n].events = EPOLLIN | EPOLLET | EPOLLRDHUP; - events[n].data.u64 = sock.sock | ((uint64_t)TCP_SOCKET_CONFIRMED << 32) | ((uint64_t)index_new << 40); + events[n].data.u64 = net_socket_to_native(sock) | ((uint64_t)TCP_SOCKET_CONFIRMED << 32) | ((uint64_t)index_new << 40); - if (epoll_ctl(tcp_server->efd, EPOLL_CTL_MOD, sock.sock, &events[n]) == -1) { + if (epoll_ctl(tcp_server->efd, EPOLL_CTL_MOD, net_socket_to_native(sock), &events[n]) == -1) { // remove from confirmed connections LOGGER_DEBUG(tcp_server->logger, "unconfirmed connection %d was dropped due to epoll error %d", index, net_error()); kill_accepted(tcp_server, index_new); diff --git a/toxcore/attributes.h b/toxcore/attributes.h index f37b5ce515..087efe20dc 100644 --- a/toxcore/attributes.h +++ b/toxcore/attributes.h @@ -26,6 +26,14 @@ #define nullable(...) +#ifdef SPARSE +#define bitwise __attribute__((bitwise)) +#define force __attribute__((force)) +#else +#define bitwise +#define force +#endif + //!TOKSTYLE+ #endif /* C_TOXCORE_TOXCORE_ATTRIBUTES_H */ diff --git a/toxcore/network.c b/toxcore/network.c index aa8b4d827f..c33c1cdac7 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -368,10 +368,20 @@ IP6 get_ip6_loopback(void) #define INVALID_SOCKET (-1) #endif /* OS_WIN32 */ +int net_socket_to_native(Socket sock) +{ + return (force int)sock.value; +} + +Socket net_socket_from_native(int sock) +{ + const Socket res = {(force Socket_Value)sock}; + return res; +} + Socket net_invalid_socket(void) { - const Socket invalid_socket = { (int)INVALID_SOCKET }; - return invalid_socket; + return net_socket_from_native(INVALID_SOCKET); } Family net_family_unspec(void) @@ -467,7 +477,7 @@ bool net_family_is_tox_tcp_ipv6(Family family) bool sock_valid(Socket sock) { const Socket invalid_socket = net_invalid_socket(); - return sock.sock != invalid_socket.sock; + return sock.value != invalid_socket.value; } struct Network_Addr { @@ -476,104 +486,104 @@ struct Network_Addr { }; non_null() -static int sys_close(void *obj, int sock) +static int sys_close(void *obj, Socket sock) { #if defined(OS_WIN32) - return closesocket(sock); + return closesocket(net_socket_to_native(sock)); #else // !OS_WIN32 - return close(sock); + return close(net_socket_to_native(sock)); #endif /* OS_WIN32 */ } non_null() -static int sys_accept(void *obj, int sock) +static Socket sys_accept(void *obj, Socket sock) { - return accept(sock, nullptr, nullptr); + return net_socket_from_native(accept(net_socket_to_native(sock), nullptr, nullptr)); } non_null() -static int sys_bind(void *obj, int sock, const Network_Addr *addr) +static int sys_bind(void *obj, Socket sock, const Network_Addr *addr) { - return bind(sock, (const struct sockaddr *)&addr->addr, addr->size); + return bind(net_socket_to_native(sock), (const struct sockaddr *)&addr->addr, addr->size); } non_null() -static int sys_listen(void *obj, int sock, int backlog) +static int sys_listen(void *obj, Socket sock, int backlog) { - return listen(sock, backlog); + return listen(net_socket_to_native(sock), backlog); } non_null() -static int sys_recvbuf(void *obj, int sock) +static int sys_recvbuf(void *obj, Socket sock) { #ifdef OS_WIN32 u_long count = 0; - ioctlsocket(sock, FIONREAD, &count); + ioctlsocket(net_socket_to_native(sock), FIONREAD, &count); #else int count = 0; - ioctl(sock, FIONREAD, &count); + ioctl(net_socket_to_native(sock), FIONREAD, &count); #endif /* OS_WIN32 */ return count; } non_null() -static int sys_recv(void *obj, int sock, uint8_t *buf, size_t len) +static int sys_recv(void *obj, Socket sock, uint8_t *buf, size_t len) { - return recv(sock, (char *)buf, len, MSG_NOSIGNAL); + return recv(net_socket_to_native(sock), (char *)buf, len, MSG_NOSIGNAL); } non_null() -static int sys_send(void *obj, int sock, const uint8_t *buf, size_t len) +static int sys_send(void *obj, Socket sock, const uint8_t *buf, size_t len) { - return send(sock, (const char *)buf, len, MSG_NOSIGNAL); + return send(net_socket_to_native(sock), (const char *)buf, len, MSG_NOSIGNAL); } non_null() -static int sys_sendto(void *obj, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) +static int sys_sendto(void *obj, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { - return sendto(sock, (const char *)buf, len, 0, (const struct sockaddr *)&addr->addr, addr->size); + return sendto(net_socket_to_native(sock), (const char *)buf, len, 0, (const struct sockaddr *)&addr->addr, addr->size); } non_null() -static int sys_recvfrom(void *obj, int sock, uint8_t *buf, size_t len, Network_Addr *addr) +static int sys_recvfrom(void *obj, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { socklen_t size = addr->size; - const int ret = recvfrom(sock, (char *)buf, len, 0, (struct sockaddr *)&addr->addr, &size); + const int ret = recvfrom(net_socket_to_native(sock), (char *)buf, len, 0, (struct sockaddr *)&addr->addr, &size); addr->size = size; return ret; } non_null() -static int sys_socket(void *obj, int domain, int type, int proto) +static Socket sys_socket(void *obj, int domain, int type, int proto) { - return (int)socket(domain, type, proto); + return net_socket_from_native(socket(domain, type, proto)); } non_null() -static int sys_socket_nonblock(void *obj, int sock, bool nonblock) +static int sys_socket_nonblock(void *obj, Socket sock, bool nonblock) { #ifdef OS_WIN32 u_long mode = nonblock ? 1 : 0; - return ioctlsocket(sock, FIONBIO, &mode); + return ioctlsocket(net_socket_to_native(sock), FIONBIO, &mode); #else - return fcntl(sock, F_SETFL, O_NONBLOCK, nonblock ? 1 : 0); + return fcntl(net_socket_to_native(sock), F_SETFL, O_NONBLOCK, nonblock ? 1 : 0); #endif /* OS_WIN32 */ } non_null() -static int sys_getsockopt(void *obj, int sock, int level, int optname, void *optval, size_t *optlen) +static int sys_getsockopt(void *obj, Socket sock, int level, int optname, void *optval, size_t *optlen) { socklen_t len = *optlen; - const int ret = getsockopt(sock, level, optname, (char *)optval, &len); + const int ret = getsockopt(net_socket_to_native(sock), level, optname, (char *)optval, &len); *optlen = len; return ret; } non_null() -static int sys_setsockopt(void *obj, int sock, int level, int optname, const void *optval, size_t optlen) +static int sys_setsockopt(void *obj, Socket sock, int level, int optname, const void *optval, size_t optlen) { - return setsockopt(sock, level, optname, (const char *)optval, optlen); + return setsockopt(net_socket_to_native(sock), level, optname, (const char *)optval, optlen); } static const Network_Funcs os_network_funcs = { @@ -623,13 +633,13 @@ void os_network_deinit(const Network *ns) non_null() static int net_setsockopt(const Network *ns, Socket sock, int level, int optname, const void *optval, size_t optlen) { - return ns->funcs->setsockopt(ns->obj, sock.sock, level, optname, optval, optlen); + return ns->funcs->setsockopt(ns->obj, sock, level, optname, optval, optlen); } non_null() static int net_getsockopt(const Network *ns, Socket sock, int level, int optname, void *optval, size_t *optlen) { - return ns->funcs->getsockopt(ns->obj, sock.sock, level, optname, optval, optlen); + return ns->funcs->getsockopt(ns->obj, sock, level, optname, optval, optlen); } non_null() @@ -804,7 +814,7 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu int net_send(const Network *ns, const Logger *log, Socket sock, const uint8_t *buf, size_t len, const IP_Port *ip_port) { - const int res = ns->funcs->send(ns->obj, sock.sock, buf, len); + const int res = ns->funcs->send(ns->obj, sock, buf, len); loglogdata(log, "T=>", buf, len, ip_port, res); return res; } @@ -814,13 +824,13 @@ static int net_sendto( const Network *ns, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr, const IP_Port *ip_port) { - return ns->funcs->sendto(ns->obj, sock.sock, buf, len, addr); + return ns->funcs->sendto(ns->obj, sock, buf, len, addr); } int net_recv(const Network *ns, const Logger *log, Socket sock, uint8_t *buf, size_t len, const IP_Port *ip_port) { - const int res = ns->funcs->recv(ns->obj, sock.sock, buf, len); + const int res = ns->funcs->recv(ns->obj, sock, buf, len); loglogdata(log, "=>T", buf, len, ip_port, res); return res; } @@ -829,35 +839,34 @@ non_null() static int net_recvfrom(const Network *ns, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { - return ns->funcs->recvfrom(ns->obj, sock.sock, buf, len, addr); + return ns->funcs->recvfrom(ns->obj, sock, buf, len, addr); } int net_listen(const Network *ns, Socket sock, int backlog) { - return ns->funcs->listen(ns->obj, sock.sock, backlog); + return ns->funcs->listen(ns->obj, sock, backlog); } non_null() static int net_bind(const Network *ns, Socket sock, const Network_Addr *addr) { - return ns->funcs->bind(ns->obj, sock.sock, addr); + return ns->funcs->bind(ns->obj, sock, addr); } Socket net_accept(const Network *ns, Socket sock) { - const Socket newsock = {ns->funcs->accept(ns->obj, sock.sock)}; - return newsock; + return ns->funcs->accept(ns->obj, sock); } /** Close the socket. */ void kill_sock(const Network *ns, Socket sock) { - ns->funcs->close(ns->obj, sock.sock); + ns->funcs->close(ns->obj, sock); } bool set_socket_nonblock(const Network *ns, Socket sock) { - return ns->funcs->socket_nonblock(ns->obj, sock.sock, true) == 0; + return ns->funcs->socket_nonblock(ns->obj, sock, true) == 0; } bool set_socket_nosigpipe(const Network *ns, Socket sock) @@ -1946,10 +1955,10 @@ bool net_connect(const Memory *mem, const Logger *log, Socket sock, const IP_Por Ip_Ntoa ip_str; LOGGER_DEBUG(log, "connecting socket %d to %s:%d", - (int)sock.sock, net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port)); + net_socket_to_native(sock), net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port)); errno = 0; - if (connect(sock.sock, (struct sockaddr *)&addr, addrsize) == -1) { + if (connect(net_socket_to_native(sock), (struct sockaddr *)&addr, addrsize) == -1) { const int error = net_error(); // Non-blocking socket: "Operation in progress" means it's connecting. @@ -2109,13 +2118,12 @@ Socket net_socket(const Network *ns, Family domain, int type, int protocol) const int platform_domain = make_family(domain); const int platform_type = make_socktype(type); const int platform_prot = make_proto(protocol); - const Socket sock = {ns->funcs->socket(ns->obj, platform_domain, platform_type, platform_prot)}; - return sock; + return ns->funcs->socket(ns->obj, platform_domain, platform_type, platform_prot); } uint16_t net_socket_data_recv_buffer(const Network *ns, Socket sock) { - const int count = ns->funcs->recvbuf(ns->obj, sock.sock); + const int count = ns->funcs->recvbuf(ns->obj, sock); return (uint16_t)max_s32(0, min_s32(count, UINT16_MAX)); } diff --git a/toxcore/network.h b/toxcore/network.h index f174ef9ebc..06857b8917 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -27,19 +27,27 @@ extern "C" { */ typedef struct Network_Addr Network_Addr; -typedef int net_close_cb(void *obj, int sock); -typedef int net_accept_cb(void *obj, int sock); -typedef int net_bind_cb(void *obj, int sock, const Network_Addr *addr); -typedef int net_listen_cb(void *obj, int sock, int backlog); -typedef int net_recvbuf_cb(void *obj, int sock); -typedef int net_recv_cb(void *obj, int sock, uint8_t *buf, size_t len); -typedef int net_recvfrom_cb(void *obj, int sock, uint8_t *buf, size_t len, Network_Addr *addr); -typedef int net_send_cb(void *obj, int sock, const uint8_t *buf, size_t len); -typedef int net_sendto_cb(void *obj, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr); -typedef int net_socket_cb(void *obj, int domain, int type, int proto); -typedef int net_socket_nonblock_cb(void *obj, int sock, bool nonblock); -typedef int net_getsockopt_cb(void *obj, int sock, int level, int optname, void *optval, size_t *optlen); -typedef int net_setsockopt_cb(void *obj, int sock, int level, int optname, const void *optval, size_t optlen); +typedef bitwise int Socket_Value; +typedef struct Socket { + Socket_Value value; +} Socket; + +int net_socket_to_native(Socket sock); +Socket net_socket_from_native(int sock); + +typedef int net_close_cb(void *obj, Socket sock); +typedef Socket net_accept_cb(void *obj, Socket sock); +typedef int net_bind_cb(void *obj, Socket sock, const Network_Addr *addr); +typedef int net_listen_cb(void *obj, Socket sock, int backlog); +typedef int net_recvbuf_cb(void *obj, Socket sock); +typedef int net_recv_cb(void *obj, Socket sock, uint8_t *buf, size_t len); +typedef int net_recvfrom_cb(void *obj, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr); +typedef int net_send_cb(void *obj, Socket sock, const uint8_t *buf, size_t len); +typedef int net_sendto_cb(void *obj, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr); +typedef Socket net_socket_cb(void *obj, int domain, int type, int proto); +typedef int net_socket_nonblock_cb(void *obj, Socket sock, bool nonblock); +typedef int net_getsockopt_cb(void *obj, Socket sock, int level, int optname, void *optval, size_t *optlen); +typedef int net_setsockopt_cb(void *obj, Socket sock, int level, int optname, const void *optval, size_t optlen); typedef int net_getaddrinfo_cb(void *obj, int family, Network_Addr **addrs); typedef int net_freeaddrinfo_cb(void *obj, Network_Addr *addrs); @@ -211,10 +219,6 @@ typedef struct IP_Port { uint16_t port; } IP_Port; -typedef struct Socket { - int sock; -} Socket; - non_null() Socket net_socket(const Network *ns, Family domain, int type, int protocol); From 969e3a2bfc833b4f6bc7126dba6c5ba368ed7493 Mon Sep 17 00:00:00 2001 From: Maxim Biro Date: Mon, 5 Feb 2024 10:50:49 -0500 Subject: [PATCH 2/2] refactor: Fix network test not using the strong typedef --- toxcore/network_test_util.cc | 26 +++++++++++++------------- toxcore/network_test_util.hh | 26 +++++++++++++------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/toxcore/network_test_util.cc b/toxcore/network_test_util.cc index 8db0136b14..fdf55f2a63 100644 --- a/toxcore/network_test_util.cc +++ b/toxcore/network_test_util.cc @@ -24,49 +24,49 @@ Network_Funcs const Network_Class::vtable = { Method::invoke<&Network_Class::freeaddrinfo>, }; -int Test_Network::close(void *obj, int sock) { return net->funcs->close(net->obj, sock); } -int Test_Network::accept(void *obj, int sock) { return net->funcs->accept(net->obj, sock); } -int Test_Network::bind(void *obj, int sock, const Network_Addr *addr) +int Test_Network::close(void *obj, Socket sock) { return net->funcs->close(net->obj, sock); } +Socket Test_Network::accept(void *obj, Socket sock) { return net->funcs->accept(net->obj, sock); } +int Test_Network::bind(void *obj, Socket sock, const Network_Addr *addr) { return net->funcs->bind(net->obj, sock, addr); } -int Test_Network::listen(void *obj, int sock, int backlog) +int Test_Network::listen(void *obj, Socket sock, int backlog) { return net->funcs->listen(net->obj, sock, backlog); } -int Test_Network::recvbuf(void *obj, int sock) { return net->funcs->recvbuf(net->obj, sock); } -int Test_Network::recv(void *obj, int sock, uint8_t *buf, size_t len) +int Test_Network::recvbuf(void *obj, Socket sock) { return net->funcs->recvbuf(net->obj, sock); } +int Test_Network::recv(void *obj, Socket sock, uint8_t *buf, size_t len) { return net->funcs->recv(net->obj, sock, buf, len); } -int Test_Network::recvfrom(void *obj, int sock, uint8_t *buf, size_t len, Network_Addr *addr) +int Test_Network::recvfrom(void *obj, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) { return net->funcs->recvfrom(net->obj, sock, buf, len, addr); } -int Test_Network::send(void *obj, int sock, const uint8_t *buf, size_t len) +int Test_Network::send(void *obj, Socket sock, const uint8_t *buf, size_t len) { return net->funcs->send(net->obj, sock, buf, len); } int Test_Network::sendto( - void *obj, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) + void *obj, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr) { return net->funcs->sendto(net->obj, sock, buf, len, addr); } -int Test_Network::socket(void *obj, int domain, int type, int proto) +Socket Test_Network::socket(void *obj, int domain, int type, int proto) { return net->funcs->socket(net->obj, domain, type, proto); } -int Test_Network::socket_nonblock(void *obj, int sock, bool nonblock) +int Test_Network::socket_nonblock(void *obj, Socket sock, bool nonblock) { return net->funcs->socket_nonblock(net->obj, sock, nonblock); } int Test_Network::getsockopt( - void *obj, int sock, int level, int optname, void *optval, size_t *optlen) + void *obj, Socket sock, int level, int optname, void *optval, size_t *optlen) { return net->funcs->getsockopt(net->obj, sock, level, optname, optval, optlen); } int Test_Network::setsockopt( - void *obj, int sock, int level, int optname, const void *optval, size_t optlen) + void *obj, Socket sock, int level, int optname, const void *optval, size_t optlen) { return net->funcs->setsockopt(net->obj, sock, level, optname, optval, optlen); } diff --git a/toxcore/network_test_util.hh b/toxcore/network_test_util.hh index 3fe91441e5..1f437d791d 100644 --- a/toxcore/network_test_util.hh +++ b/toxcore/network_test_util.hh @@ -44,22 +44,22 @@ struct Network_Class { class Test_Network : public Network_Class { const Network *net = REQUIRE_NOT_NULL(os_network()); - int close(void *obj, int sock) override; - int accept(void *obj, int sock) override; - int bind(void *obj, int sock, const Network_Addr *addr) override; - int listen(void *obj, int sock, int backlog) override; - int recvbuf(void *obj, int sock) override; - int recv(void *obj, int sock, uint8_t *buf, size_t len) override; - int recvfrom(void *obj, int sock, uint8_t *buf, size_t len, Network_Addr *addr) override; - int send(void *obj, int sock, const uint8_t *buf, size_t len) override; + int close(void *obj, Socket sock) override; + Socket accept(void *obj, Socket sock) override; + int bind(void *obj, Socket sock, const Network_Addr *addr) override; + int listen(void *obj, Socket sock, int backlog) override; + int recvbuf(void *obj, Socket sock) override; + int recv(void *obj, Socket sock, uint8_t *buf, size_t len) override; + int recvfrom(void *obj, Socket sock, uint8_t *buf, size_t len, Network_Addr *addr) override; + int send(void *obj, Socket sock, const uint8_t *buf, size_t len) override; int sendto( - void *obj, int sock, const uint8_t *buf, size_t len, const Network_Addr *addr) override; - int socket(void *obj, int domain, int type, int proto) override; - int socket_nonblock(void *obj, int sock, bool nonblock) override; + void *obj, Socket sock, const uint8_t *buf, size_t len, const Network_Addr *addr) override; + Socket socket(void *obj, int domain, int type, int proto) override; + int socket_nonblock(void *obj, Socket sock, bool nonblock) override; int getsockopt( - void *obj, int sock, int level, int optname, void *optval, size_t *optlen) override; + void *obj, Socket sock, int level, int optname, void *optval, size_t *optlen) override; int setsockopt( - void *obj, int sock, int level, int optname, const void *optval, size_t optlen) override; + void *obj, Socket sock, int level, int optname, const void *optval, size_t optlen) override; int getaddrinfo(void *obj, int family, Network_Addr **addrs) override; int freeaddrinfo(void *obj, Network_Addr *addrs) override; };