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.
TokTok · Feb 9, 2024 · 93c83fb · 93c83fb
1 parent 9fe18b1
commit 93c83fb
Show file tree

Hide file tree

Showing 13 changed files with 262 additions and 127 deletions.
diff --git 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
@@ -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
@@ -0,0 +1 @@
+!/Makefile
diff --git a/other/docker/sparse/Makefile 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
@@ -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
@@ -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
@@ -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
@@ -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<int>(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<int>(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<int>(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<int>(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<int>(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<int>(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<int>(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<int>(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
@@ -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;
         }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		CFLAGS=-O3 -g -Wno-discarded-qualifiers -Wno-format-truncation -Wno-stringop-truncation -Wno-uninitialized -Wno-unused -Wno-unused-result