Skip to content

Commit

Permalink
refactor: Use strong typedef instead of struct for Socket.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
iphydf committed Feb 4, 2024
1 parent 8b05296 commit 34cec55
Show file tree
Hide file tree
Showing 14 changed files with 210 additions and 84 deletions.
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions other/analysis/run-clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ ERRORS="*"
ERRORS="$ERRORS,-cert-err34-c"
ERRORS="$ERRORS,-readability-suspicious-call-argument"

# TODO(iphydf): Fix once cimple 0.0.19 is released.
CHECKS="$CHECKS,-google-readability-casting"

# TODO(iphydf): Fix these.
CHECKS="$CHECKS,-bugprone-switch-missing-default-case"

Expand Down
1 change: 1 addition & 0 deletions other/docker/sparse/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!/Makefile
70 changes: 70 additions & 0 deletions other/docker/sparse/Makefile
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions other/docker/sparse/local.mk
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
6 changes: 6 additions & 0 deletions other/docker/sparse/run
Original file line number Diff line number Diff line change
@@ -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" .
35 changes: 35 additions & 0 deletions other/docker/sparse/sparse.Dockerfile
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion third_party/cmp
Submodule cmp updated 1 files
+10 −10 cmp.c
4 changes: 2 additions & 2 deletions toxcore/LAN_discovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
18 changes: 9 additions & 9 deletions toxcore/TCP_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions toxcore/attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Loading

0 comments on commit 34cec55

Please sign in to comment.