From 627e18fa8ad000ed92405cff3a88c36fd5f3027e Mon Sep 17 00:00:00 2001 From: Stefano Brivio Date: Thu, 21 Oct 2021 09:41:13 +0200 Subject: [PATCH] passt: Add cppcheck target, test, and address resulting warnings ...mostly false positives, but a number of very relevant ones too, in tcp_get_sndbuf(), tcp_conn_from_tap(), and siphash PREAMBLE(). Signed-off-by: Stefano Brivio --- Makefile | 32 ++++++++++- checksum.c | 6 +- conf.c | 6 +- dhcp.c | 13 +++-- ndp.c | 8 +-- netlink.c | 8 +-- passt.c | 4 +- passt.h | 2 +- pasta.c | 11 ++-- pcap.c | 6 +- qrap.c | 5 +- siphash.c | 10 ++-- tap.c | 29 +++++----- tcp.c | 109 +++++++++++++++++++------------------ test/build/static_checkers | 6 +- udp.c | 18 +++--- util.c | 4 +- 17 files changed, 159 insertions(+), 118 deletions(-) diff --git a/Makefile b/Makefile index 9e7651b..32cc1d5 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ pkgs: # # - bugprone-suspicious-string-compare # Return value of memcmp(), not really suspicious -clang-tidy: $(wildcard *.c) +clang-tidy: $(wildcard *.c) $(wildcard *.h) clang-tidy -checks=*,-modernize-*,\ -clang-analyzer-valist.Uninitialized,\ -cppcoreguidelines-init-variables,\ @@ -163,3 +163,33 @@ clang-tidy: $(wildcard *.c) -cppcoreguidelines-avoid-non-const-global-variables,\ -bugprone-suspicious-string-compare \ --warnings-as-errors=* $(wildcard *.c) -- $(CFLAGS) + +ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1) +TARGET := $(shell ${CC} -v 2>&1 | sed -n 's/Target: \(.*\)/\1/p') +VER := $(shell $(CC) -dumpversion) +EXTRA_INCLUDES := /usr/lib/gcc/$(TARGET)/$(VER)/include +EXTRA_INCLUDES_OPT := -I$(EXTRA_INCLUDES) +else +EXTRA_INCLUDES_OPT := +endif +cppcheck: $(wildcard *.c) $(wildcard *.h) + cppcheck --std=c99 --error-exitcode=1 --enable=all --force \ + --inconclusive --library=posix \ + -I/usr/include $(EXTRA_INCLUDES_OPT) \ + --suppress=missingIncludeSystem \ + --suppress="*:$(EXTRA_INCLUDES)/avx512fintrin.h" \ + --suppress="*:$(EXTRA_INCLUDES)/xmmintrin.h" \ + --suppress="*:$(EXTRA_INCLUDES)/emmintrin.h" \ + --suppress="*:$(EXTRA_INCLUDES)/avxintrin.h" \ + --suppress="*:$(EXTRA_INCLUDES)/bmiintrin.h" \ + --suppress=objectIndex:tcp.c --suppress=objectIndex:udp.c \ + --suppress=va_list_usedBeforeStarted:util.c \ + --suppress=unusedFunction:igmp.c \ + --suppress=unusedFunction:siphash.c \ + --suppress=knownConditionTrueFalse:conf.c \ + --suppress=strtokCalled:conf.c --suppress=strtokCalled:qrap.c \ + --suppress=getpwnamCalled:passt.c \ + --suppress=localtimeCalled:pcap.c \ + --suppress=unusedStructMember:pcap.c \ + --suppress=funcArgNamesDifferent:util.h \ + . diff --git a/checksum.c b/checksum.c index dcbe905..c9905d1 100644 --- a/checksum.c +++ b/checksum.c @@ -167,8 +167,8 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init) { __m256i a, b, sum256, sum_a_hi, sum_a_lo, sum_b_hi, sum_b_lo, c, d; __m256i __sum_a_hi, __sum_a_lo, __sum_b_hi, __sum_b_lo; - const uint64_t *buf64 = (const uint64_t *)buf; - const __m256i *buf256; + const __m256i *buf256 = (const __m256i *)buf; + const uint64_t *buf64; const uint16_t *buf16; uint64_t sum64 = init; int odd = len & 1; @@ -176,7 +176,6 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init) __m256i zero; zero = _mm256_setzero_si256(); - buf256 = (const __m256i *)buf64; if (len < sizeof(__m256i) * 4) goto less_than_128_bytes; @@ -267,7 +266,6 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init) /* Fold 128-bit sum into 64 bits. */ sum64 += _mm_extract_epi64(sum128, 0) + _mm_extract_epi64(sum128, 1); - buf64 = (const uint64_t *)buf256; less_than_128_bytes: for (; len >= sizeof(a); len -= sizeof(a), buf256++) { diff --git a/conf.c b/conf.c index 7dd213d..8ec67fc 100644 --- a/conf.c +++ b/conf.c @@ -678,7 +678,7 @@ pasta_opts: void conf_print(struct ctx *c) { - char buf6[INET6_ADDRSTRLEN], buf4[INET_ADDRSTRLEN], ifn[IFNAMSIZ]; + char buf4[INET_ADDRSTRLEN], ifn[IFNAMSIZ]; int i; if (c->mode == MODE_PASTA) { @@ -723,6 +723,8 @@ void conf_print(struct ctx *c) } if (c->v6) { + char buf6[INET6_ADDRSTRLEN]; + if (!c->no_ndp && !c->no_dhcpv6) info("NDP/DHCPv6:"); else if (!c->no_ndp) @@ -1013,7 +1015,7 @@ void conf(struct ctx *c, int argc, char **argv) errno = 0; mask = strtol(optarg, NULL, 0); - if (mask >= 0 && mask <= 32 && !errno) { + if (mask > 0 && mask <= 32 && !errno) { c->mask4 = htonl(0xffffffff << (32 - mask)); break; } diff --git a/dhcp.c b/dhcp.c index 747f719..5169f56 100644 --- a/dhcp.c +++ b/dhcp.c @@ -212,19 +212,20 @@ static void opt_set_dns_search(struct ctx *c, size_t max_len) for (i = 0; *c->dns_search[i].n; i++) { unsigned int n; - int dup = -1; + int count = -1; char *p; buf[0] = 0; for (p = c->dns_search[i].n, n = 1; *p; p++) { if (*p == '.') { /* RFC 1035 4.1.4 Message compression */ - dup = opt_dns_search_dup_ptr(opts[119].s, p + 1, - opts[119].slen); + count = opt_dns_search_dup_ptr(opts[119].s, + p + 1, + opts[119].slen); - if (dup >= 0) { + if (count >= 0) { buf[n++] = '\xc0'; - buf[n++] = dup; + buf[n++] = count; break; } buf[n++] = '.'; @@ -234,7 +235,7 @@ static void opt_set_dns_search(struct ctx *c, size_t max_len) } /* The compression pointer is also an end of label */ - if (dup < 0) + if (count < 0) buf[n++] = 0; if (n >= max_len) diff --git a/ndp.c b/ndp.c index 5fa2bff..3a766ec 100644 --- a/ndp.c +++ b/ndp.c @@ -91,7 +91,7 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len) memcpy(p, c->mac, ETH_ALEN); p += 6; } else if (ih->icmp6_type == RS) { - size_t len = 0; + size_t dns_s_len = 0; int i, n; if (c->no_ra) @@ -139,7 +139,7 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len) } for (n = 0; *c->dns_search[n].n; n++) - len += strlen(c->dns_search[n].n) + 2; + dns_s_len += strlen(c->dns_search[n].n) + 2; if (len) { *p++ = 31; /* DNSSL */ *p++ = (len + 8 - 1) / 8 + 1; /* length */ @@ -163,8 +163,8 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len) *(p++) = 0; } - memset(p, 0, 8 - len % 8); /* padding */ - p += 8 - len % 8; + memset(p, 0, 8 - dns_s_len % 8); /* padding */ + p += 8 - dns_s_len % 8; } *p++ = 1; /* source ll */ diff --git a/netlink.c b/netlink.c index ca18263..54e218d 100644 --- a/netlink.c +++ b/netlink.c @@ -47,7 +47,6 @@ static int nl_seq; static int nl_sock_init_do(void *arg) { struct sockaddr_nl addr = { .nl_family = AF_NETLINK, }; - struct ctx *c = (struct ctx *)arg; int *s = &nl_sock, v = 1; ns: @@ -55,7 +54,7 @@ ns: bind(*s, (struct sockaddr *)&addr, sizeof(addr))) *s = -1; - if (*s == -1 || !c || s == &nl_sock_ns) + if (*s == -1 || !arg || s == &nl_sock_ns) return 0; setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &v, sizeof(v)); @@ -206,11 +205,10 @@ v6: word = (long *)has_v4; for (i = 0; i < ARRAY_SIZE(has_v4) / sizeof(long); i++, word++) { - int ifi; - tmp = *word; while ((n = ffsl(tmp))) { - ifi = i * sizeof(long) * 8 + n - 1; + int ifi = i * sizeof(long) * 8 + n - 1; + if (!first_v4) first_v4 = ifi; diff --git a/passt.c b/passt.c index ca4c279..6e5a72a 100644 --- a/passt.c +++ b/passt.c @@ -272,7 +272,7 @@ static void pid_file(struct ctx *c) { * @argc: Argument count * @argv: Options, plus optional target PID for pasta mode * - * Return: 0 once interrupted, non-zero on failure + * Return: non-zero on failure * * #syscalls read write open close fork dup2 exit chdir ioctl writev syslog * #syscalls prlimit64 epoll_ctl epoll_create1 epoll_wait accept4 accept listen @@ -394,6 +394,4 @@ loop: post_handler(&c, &now); goto loop; - - return 0; } diff --git a/passt.h b/passt.h index 68c42ca..ae3035f 100644 --- a/passt.h +++ b/passt.h @@ -44,7 +44,7 @@ union epoll_ref; */ union epoll_ref { struct { - uint32_t proto:8, + int32_t proto:8, s:24; union { union tcp_epoll_ref tcp; diff --git a/pasta.c b/pasta.c index c13743b..5150a3e 100644 --- a/pasta.c +++ b/pasta.c @@ -167,9 +167,8 @@ netns: */ void pasta_start_ns(struct ctx *c) { - char buf[BUFSIZ], *shell, proc_path[PATH_MAX]; - int euid = geteuid(); - int fd; + int euid = geteuid(), fd; + char *shell; c->foreground = 1; if (!c->debug) @@ -181,6 +180,8 @@ void pasta_start_ns(struct ctx *c) } if (pasta_child_pid) { + char proc_path[PATH_MAX]; + NS_CALL(pasta_wait_for_ns, c); snprintf(proc_path, PATH_MAX, "/proc/%i/ns/net", @@ -197,7 +198,9 @@ void pasta_start_ns(struct ctx *c) } if (!c->netns_only) { - snprintf(buf, BUFSIZ, "%u %u %u", 0, euid, 1); + char buf[BUFSIZ]; + + snprintf(buf, BUFSIZ, "%i %i %i", 0, euid, 1); fd = open("/proc/self/uid_map", O_WRONLY); if (write(fd, buf, strlen(buf)) < 0) diff --git a/pcap.c b/pcap.c index 2c390f2..7b5a1af 100644 --- a/pcap.c +++ b/pcap.c @@ -172,9 +172,7 @@ fail: */ void pcap_init(struct ctx *c, int index) { - char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX) ".pcap"; struct timeval tv; - struct tm *tm; if (pcap_fd != -1) return; @@ -183,6 +181,10 @@ void pcap_init(struct ctx *c, int index) return; if (*c->pcap == 1) { + char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX) + ".pcap"; + struct tm *tm; + if (c->mode == MODE_PASTA) memcpy(name, PCAP_PREFIX_PASTA, sizeof(PCAP_PREFIX_PASTA)); diff --git a/qrap.c b/qrap.c index 7b2b2b0..9a9a6ce 100644 --- a/qrap.c +++ b/qrap.c @@ -127,7 +127,7 @@ int main(int argc, char **argv) struct arphdr ah; struct arpmsg am; } probe = { - htonl(42), + .vnet_len = htonl(42), { .h_dest = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }, .h_source = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }, @@ -198,11 +198,12 @@ int main(int argc, char **argv) if (!strcmp(argv[i], "-device") && i + 1 < argc) { char *p; - long n; has_dev = 1; if ((p = strstr(argv[i + 1], dev->template))) { + long n; + n = strtol(p + strlen(dev->template), NULL, 16); if (!errno) addr_map |= (1 << n); diff --git a/siphash.c b/siphash.c index 88f8bd2..ec38848 100644 --- a/siphash.c +++ b/siphash.c @@ -65,7 +65,7 @@ int __i; \ \ do { \ - for (__i = sizeof(v) / sizeof(v[0]); __i >= 0; __i--) \ + for (__i = sizeof(v) / sizeof(v[0]) - 1; __i >= 0; __i--) \ v[__i] = k[__i % 2]; \ } while (0) @@ -152,13 +152,13 @@ __attribute__((__noinline__)) /* See comment in Makefile */ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k) { uint32_t *in32 = (uint32_t *)in; - uint64_t combined; int i; PREAMBLE(20); for (i = 0; i < 2; i++, in32 += 2) { - combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; + uint64_t combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; + v[3] ^= combined; SIPROUND(2); v[0] ^= combined; @@ -205,13 +205,13 @@ uint32_t siphash_32b(const uint8_t *in, const uint64_t *k) uint32_t siphash_36b(const uint8_t *in, const uint64_t *k) { uint32_t *in32 = (uint32_t *)in; - uint64_t combined; int i; PREAMBLE(36); for (i = 0; i < 4; i++, in32 += 2) { - combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; + uint64_t combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; + v[3] ^= combined; SIPROUND(2); v[0] ^= combined; diff --git a/tap.c b/tap.c index 57d2613..d6bad5a 100644 --- a/tap.c +++ b/tap.c @@ -265,19 +265,22 @@ static void tap_packet_debug(struct iphdr *iph, struct ipv6hdr *ip6h, { char buf6s[INET6_ADDRSTRLEN], buf6d[INET6_ADDRSTRLEN]; char buf4s[INET_ADDRSTRLEN], buf4d[INET_ADDRSTRLEN]; - uint8_t proto; + uint8_t proto = 0; if (iph || seq4) { inet_ntop(AF_INET, iph ? &iph->saddr : &seq4->saddr, - buf4s, sizeof(buf4s)), + buf4s, sizeof(buf4s)); inet_ntop(AF_INET, iph ? &iph->daddr : &seq4->daddr, - buf4d, sizeof(buf4d)), - proto = iph ? iph->protocol : seq4->protocol; + buf4d, sizeof(buf4d)); + if (iph) + proto = iph->protocol; + else if (seq4) + proto = seq4->protocol; } else { inet_ntop(AF_INET6, ip6h ? &ip6h->saddr : &seq6->saddr, - buf6s, sizeof(buf6s)), + buf6s, sizeof(buf6s)); inet_ntop(AF_INET6, ip6h ? &ip6h->daddr : &seq6->daddr, - buf6d, sizeof(buf6d)), + buf6d, sizeof(buf6d)); proto = proto6; } @@ -397,12 +400,12 @@ resume: for (seq = l4_seq4 + seq_count - 1; seq >= l4_seq4; seq--) { if (L4_MATCH(iph, uh, seq)) { if (seq->msgs >= UIO_MAXIOV) - seq = l4_seq4 - 1; + seq = NULL; break; } } - if (seq < l4_seq4) { + if (!seq || seq < l4_seq4) { seq = l4_seq4 + seq_count++; L4_SET(iph, uh, seq); seq->msgs = 0; @@ -560,12 +563,12 @@ resume: for (seq = l4_seq6 + seq_count - 1; seq >= l4_seq6; seq--) { if (L4_MATCH(ip6h, proto, uh, seq)) { if (seq->msgs >= UIO_MAXIOV) - seq = l4_seq6 - 1; + seq = NULL; break; } } - if (seq < l4_seq6) { + if (!seq || seq < l4_seq6) { seq = l4_seq6 + seq_count++; L4_SET(ip6h, proto, uh, seq); seq->msgs = 0; @@ -711,7 +714,7 @@ next: static int tap_handler_pasta(struct ctx *c, struct timespec *now) { ssize_t n = 0, len; - int err, seq4_i = 0, seq6_i = 0; + int ret, seq4_i = 0, seq6_i = 0; restart: while ((len = read(c->fd_tap, pkt_buf + n, TAP_BUF_BYTES - n)) > 0) { @@ -749,7 +752,7 @@ restart: if (len < 0 && errno == EINTR) goto restart; - err = errno; + ret = errno; if (seq4_i) tap4_handler(c, seq4, seq4_i, now); @@ -757,7 +760,7 @@ restart: if (seq6_i) tap6_handler(c, seq6, seq6_i, now); - if (len > 0 || err == EAGAIN) + if (len > 0 || ret == EAGAIN) return 0; epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL); diff --git a/tcp.c b/tcp.c index 0ff3d35..9c2881c 100644 --- a/tcp.c +++ b/tcp.c @@ -757,13 +757,14 @@ static int tcp_rtt_dst_low(struct tcp_tap_conn *conn) /** * tcp_rtt_dst_check() - Check tcpi_min_rtt, insert endpoint in table if low * @conn: Connection pointer - * @info: Pointer to struct tcp_info for socket + * @tinfo: Pointer to struct tcp_info for socket */ -static void tcp_rtt_dst_check(struct tcp_tap_conn *conn, struct tcp_info *info) +static void tcp_rtt_dst_check(struct tcp_tap_conn *conn, struct tcp_info *tinfo) { int i, hole = -1; - if (!info->tcpi_min_rtt || (int)info->tcpi_min_rtt > LOW_RTT_THRESHOLD) + if (!tinfo->tcpi_min_rtt || + (int)tinfo->tcpi_min_rtt > LOW_RTT_THRESHOLD) return; for (i = 0; i < LOW_RTT_TABLE_SIZE; i++) { @@ -809,21 +810,23 @@ static void tcp_splice_state(struct tcp_splice_conn *conn, enum tcp_state state) */ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) { - int s = conn->sock, v; + int s = conn->sock, sndbuf; socklen_t sl; + uint64_t v; - sl = sizeof(v); - if (getsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, &sl)) { + sl = sizeof(sndbuf); + if (getsockopt(s, SOL_SOCKET, SO_SNDBUF, &sndbuf, &sl)) { conn->snd_buf = WINDOW_DEFAULT; return; } + v = sndbuf; if (v >= SNDBUF_BIG) v /= 2; else if (v > SNDBUF_SMALL) v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2; - conn->snd_buf = v; + conn->snd_buf = MIN(INT_MAX, v); } /** @@ -1537,17 +1540,17 @@ static size_t tcp_l2_buf_fill_headers(struct ctx *c, struct tcp_tap_conn *conn, * @c: Execution context * @conn: Connection pointer * @flags: TCP header flags we are about to send, if any - * @info: tcp_info from kernel, can be NULL if not pre-fetched + * @tinfo: tcp_info from kernel, can be NULL if not pre-fetched * * Return: 1 if sequence or window were updated, 0 otherwise */ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn, - int flags, struct tcp_info *info) + int flags, struct tcp_info *tinfo) { uint32_t prev_ack_to_tap = conn->seq_ack_to_tap; uint32_t prev_wnd_to_tap = conn->wnd_to_tap; - socklen_t sl = sizeof(*info); - struct tcp_info info_new; + socklen_t sl = sizeof(*tinfo); + struct tcp_info tinfo_new; int s = conn->sock; if (conn->state > ESTABLISHED || (flags & (DUP_ACK | FORCE_ACK)) || @@ -1555,13 +1558,13 @@ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn, conn->snd_buf < SNDBUF_SMALL) { conn->seq_ack_to_tap = conn->seq_from_tap; } else if (conn->seq_ack_to_tap != conn->seq_from_tap) { - if (!info) { - info = &info_new; - if (getsockopt(s, SOL_TCP, TCP_INFO, info, &sl)) + if (!tinfo) { + tinfo = &tinfo_new; + if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) return 0; } - conn->seq_ack_to_tap = info->tcpi_bytes_acked + + conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked + conn->seq_init_from_tap; if (SEQ_LT(conn->seq_ack_to_tap, prev_ack_to_tap)) @@ -1574,20 +1577,20 @@ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn, goto out; } - if (!info) { + if (!tinfo) { if (conn->wnd_to_tap > WINDOW_DEFAULT) goto out; - info = &info_new; - if (getsockopt(s, SOL_TCP, TCP_INFO, info, &sl)) + tinfo = &tinfo_new; + if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) goto out; } if (conn->local || tcp_rtt_dst_low(conn)) { - conn->wnd_to_tap = info->tcpi_snd_wnd; + conn->wnd_to_tap = tinfo->tcpi_snd_wnd; } else { tcp_get_sndbuf(conn); - conn->wnd_to_tap = MIN((int)info->tcpi_snd_wnd, conn->snd_buf); + conn->wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, conn->snd_buf); } conn->wnd_to_tap = MIN(conn->wnd_to_tap, MAX_WINDOW); @@ -1613,8 +1616,8 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags, uint32_t prev_wnd_to_tap = conn->wnd_to_tap; struct tcp4_l2_flags_buf_t *b4 = NULL; struct tcp6_l2_flags_buf_t *b6 = NULL; - struct tcp_info info = { 0 }; - socklen_t sl = sizeof(info); + struct tcp_info tinfo = { 0 }; + socklen_t sl = sizeof(tinfo); size_t optlen = 0, eth_len; int s = conn->sock; struct iovec *iov; @@ -1626,15 +1629,15 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags, !flags && conn->wnd_to_tap) return 0; - if (getsockopt(s, SOL_TCP, TCP_INFO, &info, &sl)) { + if (getsockopt(s, SOL_TCP, TCP_INFO, &tinfo, &sl)) { tcp_tap_destroy(c, conn); return -ECONNRESET; } if (!conn->local) - tcp_rtt_dst_check(conn, &info); + tcp_rtt_dst_check(conn, &tinfo); - if (!tcp_update_seqack_wnd(c, conn, flags, &info) && !flags) + if (!tcp_update_seqack_wnd(c, conn, flags, &tinfo) && !flags) return 0; if (CONN_V4(conn)) { @@ -1661,7 +1664,7 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags, *data++ = OPT_MSS_LEN; if (c->mtu == -1) { - mss = info.tcpi_snd_mss; + mss = tinfo.tcpi_snd_mss; } else { mss = c->mtu - sizeof(struct tcphdr); if (CONN_V4(conn)) @@ -1681,11 +1684,11 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags, th->doff += OPT_MSS_LEN / 4; #ifdef HAS_SND_WND - if (!c->tcp.kernel_snd_wnd && info.tcpi_snd_wnd) + if (!c->tcp.kernel_snd_wnd && tinfo.tcpi_snd_wnd) c->tcp.kernel_snd_wnd = 1; #endif - conn->ws = MIN(MAX_WS, info.tcpi_snd_wscale); + conn->ws = MIN(MAX_WS, tinfo.tcpi_snd_wscale); *data++ = OPT_NOP; *data++ = OPT_WS; @@ -1768,7 +1771,7 @@ static void tcp_rst(struct ctx *c, struct tcp_tap_conn *conn) static void tcp_clamp_window(struct tcp_tap_conn *conn, struct tcphdr *th, int len, unsigned int window, int init) { - if (init) { + if (init && th) { int ws = tcp_opt_get(th, len, OPT_WS, NULL, NULL); conn->ws_tap = ws; @@ -1901,7 +1904,7 @@ static void tcp_conn_from_tap(struct ctx *c, int af, void *addr, sock_pool_p = &init_sock_pool6[i]; else sock_pool_p = &init_sock_pool4[i]; - if ((ref.r.s = s = *sock_pool_p) >= 0) { + if ((ref.r.s = s = (*sock_pool_p)) >= 0) { *sock_pool_p = -1; break; } @@ -2164,7 +2167,7 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn, struct timespec *now) { int fill_bufs, send_bufs = 0, last_len, iov_rem = 0; - int send, len, plen, v4 = CONN_V4(conn); + int sendlen, len, plen, v4 = CONN_V4(conn); uint32_t seq_to_tap = conn->seq_to_tap; int s = conn->sock, i, ret = 0; struct msghdr mh_sock = { 0 }; @@ -2226,16 +2229,16 @@ recvmsg: if (!len) goto zero_len; - send = len - already_sent; - if (send <= 0) { + sendlen = len - already_sent; + if (sendlen <= 0) { tcp_tap_epoll_mask(c, conn, conn->events | EPOLLET); return 0; } tcp_tap_epoll_mask(c, conn, conn->events & ~EPOLLET); - send_bufs = DIV_ROUND_UP(send, conn->mss_guest); - last_len = send - (send_bufs - 1) * conn->mss_guest; + send_bufs = DIV_ROUND_UP(sendlen, conn->mss_guest); + last_len = sendlen - (send_bufs - 1) * conn->mss_guest; /* Likely, some new data was acked too. */ tcp_update_seqack_wnd(c, conn, 0, NULL); @@ -2594,7 +2597,7 @@ int tcp_tap_handler(struct ctx *c, int af, void *addr, conn->mss_guest = MIN(MSS6, conn->mss_guest); } - /* info.tcpi_bytes_acked already includes one byte for SYN, but + /* tinfo.tcpi_bytes_acked already includes one byte for SYN, but * not for incoming connections. */ conn->seq_init_from_tap = ntohl(th->seq) + 1; @@ -2787,8 +2790,8 @@ static int tcp_splice_connect(struct ctx *c, struct tcp_splice_conn *conn, .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) }, }; const struct sockaddr *sa; - int ret, one = 1; socklen_t sl; + int one = 1; conn->to = sock_conn; @@ -2807,7 +2810,8 @@ static int tcp_splice_connect(struct ctx *c, struct tcp_splice_conn *conn, if (connect(conn->to, sa, sl)) { if (errno != EINPROGRESS) { - ret = -errno; + int ret = -errno; + close(sock_conn); return ret; } @@ -3049,10 +3053,8 @@ void tcp_sock_handler_splice(struct ctx *c, union epoll_ref ref, goto close; if (events & EPOLLOUT) { - struct epoll_event ev = { - .events = EPOLLIN | EPOLLRDHUP, - .data.u64 = ref.u64, - }; + ev.events = EPOLLIN | EPOLLRDHUP; + ev.data.u64 = ref.u64; if (conn->state == SPLICE_CONNECT) tcp_splice_connect_finish(c, conn, ref.r.p.tcp.tcp.v6); @@ -3111,12 +3113,13 @@ swap: while (1) { int retry_write = 0, more = 0; - ssize_t read, to_write = 0, written; + ssize_t readlen, to_write = 0, written; retry: - read = splice(move_from, NULL, pipes[1], NULL, c->tcp.pipe_size, - SPLICE_F_MOVE | SPLICE_F_NONBLOCK); - if (read < 0) { + readlen = splice(move_from, NULL, pipes[1], NULL, + c->tcp.pipe_size, + SPLICE_F_MOVE | SPLICE_F_NONBLOCK); + if (readlen < 0) { if (errno == EINTR) goto retry; @@ -3124,13 +3127,13 @@ retry: goto close; to_write = c->tcp.pipe_size; - } else if (!read) { + } else if (!readlen) { eof = 1; to_write = c->tcp.pipe_size; } else { never_read = 0; - to_write += read; - if (read >= (long)c->tcp.pipe_size * 90 / 100) + to_write += readlen; + if (readlen >= (long)c->tcp.pipe_size * 90 / 100) more = SPLICE_F_MORE; if (bitmap_isset(rcvlowat_set, conn - ts)) @@ -3142,12 +3145,12 @@ eintr: SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); /* Most common case: skip updating counters. */ - if (read > 0 && read == written) { - if (read >= (long)c->tcp.pipe_size * 10 / 100) + if (readlen > 0 && readlen == written) { + if (readlen >= (long)c->tcp.pipe_size * 10 / 100) continue; if (!bitmap_isset(rcvlowat_set, conn - ts) && - read > (long)c->tcp.pipe_size / 10) { + readlen > (long)c->tcp.pipe_size / 10) { int lowat = c->tcp.pipe_size / 4; setsockopt(move_from, SOL_SOCKET, SO_RCVLOWAT, @@ -3160,7 +3163,7 @@ eintr: break; } - *seq_read += read > 0 ? read : 0; + *seq_read += readlen > 0 ? readlen : 0; *seq_write += written > 0 ? written : 0; if (written < 0) { diff --git a/test/build/static_checkers b/test/build/static_checkers index 6e080b4..d18dea8 100644 --- a/test/build/static_checkers +++ b/test/build/static_checkers @@ -11,8 +11,12 @@ # Copyright (c) 2021 Red Hat GmbH # Author: Stefano Brivio -htools clang-tidy +htools clang-tidy cppcheck test Run clang-tidy hout RET make clang-tidy; echo $? check [ __RET__ -eq 0 ] + +test Run cppcheck +hout RET make cppcheck; echo $? +check [ __RET__ -eq 0 ] diff --git a/udp.c b/udp.c index 2f79297..3b8a70a 100644 --- a/udp.c +++ b/udp.c @@ -616,9 +616,9 @@ static void udp_sock_handler_splice(struct ctx *c, union epoll_ref ref, if (ref.r.p.udp.udp.splice == UDP_TO_NS || ref.r.p.udp.udp.splice == UDP_TO_INIT) { for (i = 0; i < n; i++) { - struct msghdr *mh = &udp_splice_mmh_send[i].msg_hdr; + struct msghdr *mh_s = &udp_splice_mmh_send[i].msg_hdr; - mh->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; + mh_s->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; } sendmmsg(s, udp_splice_mmh_send, n, MSG_NOSIGNAL); @@ -626,9 +626,9 @@ static void udp_sock_handler_splice(struct ctx *c, union epoll_ref ref, } for (i = 0; i < n; i++) { - struct msghdr *mh = &udp_splice_mmh_sendto[i].msg_hdr; + struct msghdr *mh_s = &udp_splice_mmh_sendto[i].msg_hdr; - mh->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; + mh_s->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; } if (v6) { @@ -710,8 +710,6 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, else b->ip6h.saddr = c->addr6_ll; - b->ip6h.saddr = c->gw6; - udp_tap_map[V6][src].ts_local = now->tv_sec; if (IN6_IS_ADDR_LOOPBACK(&b->s_in6.sin6_addr)) @@ -1000,11 +998,11 @@ int udp_tap_handler(struct ctx *c, int af, void *addr, } for (i = 0; i < count; i++) { - struct udphdr *uh; + struct udphdr *uh_send; - uh = (struct udphdr *)(msg[i].pkt_buf_offset + pkt_buf); - m[i].iov_base = (char *)(uh + 1); - m[i].iov_len = msg[i].l4_len - sizeof(*uh); + uh_send = (struct udphdr *)(msg[i].pkt_buf_offset + pkt_buf); + m[i].iov_base = (char *)(uh_send + 1); + m[i].iov_len = msg[i].l4_len - sizeof(*uh_send); mm[i].msg_hdr.msg_name = sa; mm[i].msg_hdr.msg_namelen = sl; diff --git a/util.c b/util.c index 5ecb43a..3c4ba33 100644 --- a/util.c +++ b/util.c @@ -51,7 +51,7 @@ void name(const char *format, ...) { \ \ if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) { \ clock_gettime(CLOCK_REALTIME, &tp); \ - fprintf(stderr, "%lu.%04lu: ", \ + fprintf(stderr, "%li.%04li: ", \ tp.tv_sec - log_debug_start, \ tp.tv_nsec / (100 * 1000)); \ } else { \ @@ -142,7 +142,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) if (format[strlen(format)] != '\n') n += snprintf(buf + n, BUFSIZ - n, "\n"); - if (log_opt | LOG_PERROR) + if (log_opt & LOG_PERROR) fprintf(stderr, "%s", buf + sizeof("<0>")); send(log_sock, buf, n, 0);