From patchwork Tue Jul 15 14:37:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4309 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:8e92:b0:671:5a2c:6455 with SMTP id kd18csp2029857mab; Tue, 15 Jul 2025 07:38:12 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVOMcU37J5HTpjBTPVWS0pcPEKfzv2uSZoMJrAo96oUYW8KjC0O/0U9ec74GSGizc94US3Lun0asLY=@openvpn.net X-Google-Smtp-Source: AGHT+IEQljLeWQ2OzROA+4xnksTJDFDeENP4pC0vk296h8eKyW9rNi89WIzAuT+R9Fjqb/a9F6Fz X-Received: by 2002:a05:6830:6c08:b0:72a:449e:2b69 with SMTP id 46e09a7af769-73cf9ef05a0mr11830535a34.28.1752590292054; Tue, 15 Jul 2025 07:38:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1752590292; cv=none; d=google.com; s=arc-20240605; b=Vf4bkAxtuPUB+x7TI/m0TYa117hgB+41wkC5mGix4flfkQXScD54uM0Nt0Ygic/TWv ypdR6vcmMDB0WcMmGMHn6z91Avn/sF4QTTd9nF/CLwSRDhZQVC6ye1TFidKf7cgzG+4W Go+5+z2HiPOxy/pQNeNGn+4In9l8TgFndfc/yQ7/kjtQ/jXj6TzV6lPb/57U5at4n3mz Eueh/qrEMyk2i70XSpsQ+OWUxUdrK5h1ySG3mmEBP6oEUxhMaJpsAKlGoHihODNIQwEl tZYMJDrKVorgGjxF1YA/lS7cVZphuHISaZKQQyxDZZJYeeJep/Klbyo2Ww17aoqluuI5 kFsA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=ndmangGZTOFVFJa3SXwJhKkj52km2CUXgRnEO17SVnM=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=QF4SLRwU5wm9kz+AxWNtmN5X1CWHQwfZPfDckYz0Bb8oLuJicnbryRdDqJJta6nNU1 nDFwu4CwHwrptXM8YkEZoYBNylzydQTySCcHz960MWkRkumzPjEBFMv9xsiNoXNtLDhh 01qUiv57f8d1EmgyLtSEMQdAxCtuUqnLj4sD7adkYMXwiEthAQcgpw9he8Po7VShReC5 wHsLsLo1u507wZHt+fCaHQCAlvZk/iO48M8Se4fQQEpkGhZpYYXBaVN/xVrLseIg5K4Z jFIpCwjKYlCKkLBJPImFAcUjz/N5fBtZXnvcgW78Og/kVC7+s+a5Aovv27Yia+gakGA5 G2ZA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=XacURG9u; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=fNKqBgPA; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=EC8+R3Fn; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-73cf12a74fdsi6359892a34.219.2025.07.15.07.38.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Jul 2025 07:38:11 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=XacURG9u; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=fNKqBgPA; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=EC8+R3Fn; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ndmangGZTOFVFJa3SXwJhKkj52km2CUXgRnEO17SVnM=; b=XacURG9ui7z15sRgfo1Yg7Zu77 bYybx+nfXpO+cyjbYUms81QV/J2KKfN2n6kCRZCrl5FsSauZ0rC0Zs8eBNr/vMWYVisxay7FCJE54 208hUJ/TBb9VgIdZorYk6pyAdZ8N5dGN1vC2fLRT+6lAu9y0BUuX4UPjafNkZx0ktMtQ=; Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1ubgnH-0006gD-Ut; Tue, 15 Jul 2025 14:38:08 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1ubgnG-0006fx-B1 for openvpn-devel@lists.sourceforge.net; Tue, 15 Jul 2025 14:38:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=SD0SiV36YL91o/VKTcLOKaVluXO4/qSIK9Rn7gLWGXE=; b=fNKqBgPA4DiWeFnUIpi33W5vcc EdmvX1VoBpTgYYPnXtSBuzd1iu8YwtXxLZwL+M44UvWD3+Qla54uQ20NPnpo61XTqNLnnv1AUpca2 hddvDUUntX070cWxjsDo7e0FG/uWD+uWl199az1uGCwlwu8M343brbOmhdM9Ob+6nplE=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=SD0SiV36YL91o/VKTcLOKaVluXO4/qSIK9Rn7gLWGXE=; b=EC8+R3FnrM7/PgoNnGoq+4Jn+y FwKpSyXlVdfLN8NPvwnqFSpv9gSM6nRDMq7lpj4rkODFsQ8AOeNLH2y7o6VVTypFZbPQz3U6JqlIS BNz90rxbHwRDQYXRWdhnT3QZIRpTG13lkmxRFH56P2Q6WF6J9kcb9UdL7VVZkwxs6R84=; Received: from [193.149.48.143] (helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1ubgnF-00042q-P3 for openvpn-devel@lists.sourceforge.net; Tue, 15 Jul 2025 14:38:07 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.17.1.9/8.17.1.9) with ESMTP id 56FEbsei009749 for ; Tue, 15 Jul 2025 16:37:54 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.17.1.9/8.17.1.9/Submit) id 56FEbsFL009748 for openvpn-devel@lists.sourceforge.net; Tue, 15 Jul 2025 16:37:54 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Tue, 15 Jul 2025 16:37:44 +0200 Message-ID: <20250715143750.9719-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 1.3 (+) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-2.hosts.colo.sdot.me", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Frank Lichtenheld Check for unused objects (in reliable_get_num_output_sequenced_available) and missing free (in reliable_can_get). While looking through the code, modernize the loop variable usage. Content analysis details: (1.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS X-Headers-End: 1ubgnF-00042q-P3 Subject: [Openvpn-devel] [PATCH v3] reliable: Review and fix gc_arena usage X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1837724118083223926?= X-GMAIL-MSGID: =?utf-8?q?1837724118083223926?= From: Frank Lichtenheld Check for unused objects (in reliable_get_num_output_sequenced_available) and missing free (in reliable_can_get). While looking through the code, modernize the loop variable usage. Change-Id: I8cefa9a406fe90bb3cbe481304782c639691a3a0 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1083 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c index 424d194..5505f56 100644 --- a/src/openvpn/reliable.c +++ b/src/openvpn/reliable.c @@ -98,8 +98,7 @@ static inline bool reliable_ack_packet_id_present(struct reliable_ack *ack, packet_id_type pid) { - int i; - for (i = 0; i < ack->len; ++i) + for (int i = 0; i < ack->len; ++i) { if (ack->packet_id[i] == pid) { @@ -312,10 +311,7 @@ const char * reliable_ack_print(struct buffer *buf, bool verbose, struct gc_arena *gc) { - int i; uint8_t n_ack; - struct session_id sid_ack; - packet_id_type pid; struct buffer out = alloc_buf_gc(256, gc); buf_printf(&out, "["); @@ -323,8 +319,9 @@ { goto done; } - for (i = 0; i < n_ack; ++i) + for (int i = 0; i < n_ack; ++i) { + packet_id_type pid; if (!buf_read(buf, &pid, sizeof(pid))) { goto done; @@ -334,6 +331,7 @@ } if (n_ack) { + struct session_id sid_ack; if (!session_id_read(&sid_ack, buf)) { goto done; @@ -356,14 +354,12 @@ void reliable_init(struct reliable *rel, int buf_size, int offset, int array_size, bool hold) { - int i; - CLEAR(*rel); ASSERT(array_size > 0 && array_size <= RELIABLE_CAPACITY); rel->hold = hold; rel->size = array_size; rel->offset = offset; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; e->buf = alloc_buf(buf_size); @@ -378,8 +374,7 @@ { return; } - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; free_buf(&e->buf); @@ -391,8 +386,7 @@ bool reliable_empty(const struct reliable *rel) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -407,11 +401,10 @@ void reliable_send_purge(struct reliable *rel, const struct reliable_ack *ack) { - int i, j; - for (i = 0; i < ack->len; ++i) + for (int i = 0; i < ack->len; ++i) { packet_id_type pid = ack->packet_id[i]; - for (j = 0; j < rel->size; ++j) + for (int j = 0; j < rel->size; ++j) { struct reliable_entry *e = &rel->array[j]; if (e->active && e->packet_id == pid) @@ -449,10 +442,9 @@ reliable_print_ids(const struct reliable *rel, struct gc_arena *gc) { struct buffer out = alloc_buf_gc(256, gc); - int i; buf_printf(&out, "[" packet_id_format "]", (packet_id_print_type)rel->packet_id); - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -468,9 +460,7 @@ bool reliable_can_get(const struct reliable *rel) { - struct gc_arena gc = gc_new(); - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (!e->active) @@ -478,6 +468,7 @@ return true; } } + struct gc_arena gc = gc_new(); dmsg(D_REL_LOW, "ACK no free receive buffer available: %s", reliable_print_ids(rel, &gc)); gc_free(&gc); return false; @@ -488,12 +479,11 @@ reliable_not_replay(const struct reliable *rel, packet_id_type id) { struct gc_arena gc = gc_new(); - int i; if (reliable_pid_min(id, rel->packet_id)) { goto bad; } - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active && e->packet_id == id) @@ -514,19 +504,17 @@ bool reliable_wont_break_sequentiality(const struct reliable *rel, packet_id_type id) { - struct gc_arena gc = gc_new(); - const int ret = reliable_pid_in_range2(id, rel->packet_id, rel->size); if (!ret) { + struct gc_arena gc = gc_new(); dmsg(D_REL_LOW, "ACK " packet_id_format " breaks sequentiality: %s", (packet_id_print_type)id, reliable_print_ids(rel, &gc)); + gc_free(&gc); } dmsg(D_REL_DEBUG, "ACK RWBS rel->size=%d rel->packet_id=%08x id=%08x ret=%d", rel->size, rel->packet_id, id, ret); - - gc_free(&gc); return ret; } @@ -534,8 +522,7 @@ struct buffer * reliable_get_buf(struct reliable *rel) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (!e->active) @@ -550,7 +537,6 @@ int reliable_get_num_output_sequenced_available(struct reliable *rel) { - struct gc_arena gc = gc_new(); packet_id_type min_id = 0; bool min_id_defined = false; @@ -573,7 +559,6 @@ { ret -= subtract_pid(rel->packet_id, min_id); } - gc_free(&gc); return ret; } @@ -581,14 +566,12 @@ struct buffer * reliable_get_buf_output_sequenced(struct reliable *rel) { - struct gc_arena gc = gc_new(); - int i; packet_id_type min_id = 0; bool min_id_defined = false; struct buffer *ret = NULL; /* find minimum active packet_id */ - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -607,9 +590,10 @@ } else { + struct gc_arena gc = gc_new(); dmsg(D_REL_LOW, "ACK output sequence broken: %s", reliable_print_ids(rel, &gc)); + gc_free(&gc); } - gc_free(&gc); return ret; } @@ -617,8 +601,7 @@ struct reliable_entry * reliable_get_entry_sequenced(struct reliable *rel) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (e->active && e->packet_id == rel->packet_id) @@ -634,9 +617,8 @@ reliable_can_send(const struct reliable *rel) { struct gc_arena gc = gc_new(); - int i; int n_active = 0, n_current = 0; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -662,11 +644,10 @@ struct buffer * reliable_send(struct reliable *rel, int *opcode) { - int i; struct reliable_entry *best = NULL; const time_t local_now = now; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; @@ -701,10 +682,9 @@ void reliable_schedule_now(struct reliable *rel) { - int i; dmsg(D_REL_DEBUG, "ACK reliable_schedule_now"); rel->hold = false; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -722,10 +702,9 @@ { struct gc_arena gc = gc_new(); interval_t ret = BIG_TIMEOUT; - int i; const time_t local_now = now; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) @@ -758,8 +737,7 @@ reliable_mark_active_incoming(struct reliable *rel, struct buffer *buf, packet_id_type pid, int opcode) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (buf == &e->buf) @@ -790,8 +768,7 @@ void reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, int opcode) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (buf == &e->buf) @@ -817,8 +794,7 @@ void reliable_mark_deleted(struct reliable *rel, struct buffer *buf) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (buf == &e->buf) @@ -836,10 +812,8 @@ void reliable_ack_debug_print(const struct reliable_ack *ack, char *desc) { - int i; - printf("********* struct reliable_ack %s\n", desc); - for (i = 0; i < ack->len; ++i) + for (int i = 0; i < ack->len; ++i) { printf(" %d: " packet_id_format "\n", i, (packet_id_print_type) ack->packet_id[i]); } @@ -848,14 +822,13 @@ void reliable_debug_print(const struct reliable *rel, char *desc) { - int i; update_time(); printf("********* struct reliable %s\n", desc); printf(" initial_timeout=%d\n", (int)rel->initial_timeout); printf(" packet_id=" packet_id_format "\n", rel->packet_id); printf(" now=%" PRIi64 "\n", (int64_t)now); - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active)