From patchwork Sat Aug 13 02:04:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 2667 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director14.mail.ord1d.rsapps.net ([172.27.255.55]) by backend30.mail.ord1d.rsapps.net with LMTP id +MT8LHeT92J3QAAAIUCqbw (envelope-from ) for ; Sat, 13 Aug 2022 08:05:11 -0400 Received: from proxy16.mail.iad3a.rsapps.net ([172.27.255.55]) by director14.mail.ord1d.rsapps.net with LMTP id gGq7LHeT92L0WwAAeJ7fFg (envelope-from ) for ; Sat, 13 Aug 2022 08:05:11 -0400 Received: from smtp49.gate.iad3a ([172.27.255.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy16.mail.iad3a.rsapps.net with LMTPS id YIHpJXeT92JvCQAADc5QwQ (envelope-from ) for ; Sat, 13 Aug 2022 08:05:11 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp49.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=unstable.cc; dmarc=fail (p=none; dis=none) header.from=unstable.cc X-Suspicious-Flag: YES X-Classification-ID: 2e0bd032-1b00-11ed-8e67-525400fffce0-1-1 Received: from [216.105.38.7] ([216.105.38.7:59460] helo=lists.sourceforge.net) by smtp49.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id E0/35-19501-77397F26; Sat, 13 Aug 2022 08:05:11 -0400 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 1oMps5-0001IZ-Ao; Sat, 13 Aug 2022 12:04:05 +0000 Received: from [172.30.20.202] (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 1oMprw-0001IA-D3 for openvpn-devel@lists.sourceforge.net; Sat, 13 Aug 2022 12:03:59 +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:Message-Id: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=WFltnBIsb/iBmrQRYdbk+wQXTkaqJK0gFLmyGu7KprI=; b=dK0Sa2PC819amWYQMU3eKOXkeD 8Z/w0Web43vLZgs6YlP+qukzXQlgvfe3o1ywcCflTboprgyvUkNbMoG+cusv6ssTlTddIwyKa9hLP rdV5eMQEEYDL45iXlF8XzXKhMHtucIQukz0eEV5Y5VjGNC09AFuz7FRdihxsPGSASLGM=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:Cc:To:From :Sender:Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=WFltnBIsb/iBmrQRYdbk+wQXTkaqJK0gFLmyGu7KprI=; b=D YPVc9Fmr1LOsEarjKzb4XE2AR0IBXqFJJgdAWgw8rMdgpSMjl1sYBpkXKtauANOuFmBUdBuAtgrXe SrfmMBOztWEWI+PaegkB42HMR0wBDbhHQImssf3AfW+vWbiTSdar1evv9I0RCEcHL2lC35h5gwQS6 7W4NlzNFIJV5fBhs=; Received: from wilbur.contactoffice.com ([212.3.242.68]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1oMprm-000gNj-HN for openvpn-devel@lists.sourceforge.net; Sat, 13 Aug 2022 12:03:56 +0000 Received: from smtpauth2.co-bxl (smtpauth2.co-bxl [10.2.0.24]) by wilbur.contactoffice.com (Postfix) with ESMTP id 88133295A; Sat, 13 Aug 2022 14:03:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1660392219; s=20220809-q8oc; d=unstable.cc; i=a@unstable.cc; h=From:Cc:Date:Message-Id:MIME-Version:Content-Transfer-Encoding; l=6811; bh=WFltnBIsb/iBmrQRYdbk+wQXTkaqJK0gFLmyGu7KprI=; b=QPE0JU0mQlvyAv1MR7RQT+bTUvEh1LAsHgDTX17eoCuN/jqEC2W9vlYboZx6yusk pTN0oTFa4f7NR7/uYZpkobUtLONlhREzs/ONHPzVdLUs7hL4bbuiRIittJwfV2tt01A oZLUWL+euJst9QyPbUHDfqo1D32X2gbegxhQ11cWWn20F1etPgQ7aPLB2NHoS9Vf7/S rz56YPqfh7dwAxVDW7kNYhBl/sGHPdYpfnodb9XWw25s53Hie5xW3DZMYAOBAzWSvw/ 8I+SVASQZzAcEx1M5wluUmprQlBHh3yruihyQDV4fMwfDGFgpsYa+lRizwLIwilJaH4 2HWRj5pfPQ== Received: by smtp.mailfence.com with ESMTPSA ; Sat, 13 Aug 2022 14:03:36 +0200 (CEST) From: Antonio Quartulli To: openvpn-devel@lists.sourceforge.net Date: Sat, 13 Aug 2022 14:04:28 +0200 Message-Id: <20220813120428.6767-1-a@unstable.cc> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 X-Spam-Status: No, hits=-2.9 required=4.7 symbols=ALL_TRUSTED, BAYES_00, T_SCC_BODY_TEXT_LINE device=10.2.0.21 X-ContactOffice-Account: com:375058688 X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", 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: OpenVPN often uses a multi-indentation pattern with no real gain: if (a) { if (b) { ... } } This approach makes the code harder to read because a lot of space is eaten by indentation. Content analysis details: (-0.9 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [212.3.242.68 listed in list.dnswl.org] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-Headers-End: 1oMprm-000gNj-HN Subject: [Openvpn-devel] [PATCH] do_close_tun: get rid of one level of indentation 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: , Cc: Antonio Quartulli Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox OpenVPN often uses a multi-indentation pattern with no real gain: if (a) { if (b) { ... } } This approach makes the code harder to read because a lot of space is eaten by indentation. Cases like this can be easily converted by negating the first condition and exiting immediately: if (!a) { return; } if (b) { ... } Apply this change to do_close_tun() only for now in order to make the functiona bit easier to read. Ideally, this approach should be adopted for other parts of the code as well. NOTE: this patch is better viewed with "git show -w" as the real change is only about 3 lines. The rest is indentation change. Signed-off-by: Antonio Quartulli Acked-by: Gert Doering --- ** the dco-win patchset is based on this patch. I should have sent this earlier, but it slipped off. src/openvpn/init.c | 174 +++++++++++++++++++++++---------------------- 1 file changed, 88 insertions(+), 86 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index d67bc5d1..82a57bef 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1924,65 +1924,38 @@ do_close_tun_simple(struct context *c) static void do_close_tun(struct context *c, bool force) { - struct gc_arena gc = gc_new(); - if (c->c1.tuntap && c->c1.tuntap_owned) + if (!c->c1.tuntap || !c->c1.tuntap_owned) { - const char *tuntap_actual = string_alloc(c->c1.tuntap->actual_name, &gc); + return; + } + + struct gc_arena gc = gc_new(); + const char *tuntap_actual = string_alloc(c->c1.tuntap->actual_name, &gc); #ifdef _WIN32 - DWORD adapter_index = c->c1.tuntap->adapter_index; + DWORD adapter_index = c->c1.tuntap->adapter_index; #endif - const in_addr_t local = c->c1.tuntap->local; - const in_addr_t remote_netmask = c->c1.tuntap->remote_netmask; + const in_addr_t local = c->c1.tuntap->local; + const in_addr_t remote_netmask = c->c1.tuntap->remote_netmask; - if (force || !(c->sig->signal_received == SIGUSR1 && c->options.persist_tun)) - { - static_context = NULL; + if (force || !(c->sig->signal_received == SIGUSR1 && c->options.persist_tun)) + { + static_context = NULL; #ifdef ENABLE_MANAGEMENT - /* tell management layer we are about to close the TUN/TAP device */ - if (management) - { - management_pre_tunnel_close(management); - management_up_down(management, "DOWN", c->c2.es); - } -#endif - - /* delete any routes we added */ - if (c->c1.route_list || c->c1.route_ipv6_list) - { - run_up_down(c->options.route_predown_script, - c->plugins, - OPENVPN_PLUGIN_ROUTE_PREDOWN, - tuntap_actual, -#ifdef _WIN32 - adapter_index, + /* tell management layer we are about to close the TUN/TAP device */ + if (management) + { + management_pre_tunnel_close(management); + management_up_down(management, "DOWN", c->c2.es); + } #endif - NULL, - c->c2.frame.tun_mtu, - print_in_addr_t(local, IA_EMPTY_IF_UNDEF, &gc), - print_in_addr_t(remote_netmask, IA_EMPTY_IF_UNDEF, &gc), - "init", - signal_description(c->sig->signal_received, - c->sig->signal_text), - "route-pre-down", - c->c2.es); - - delete_routes(c->c1.route_list, c->c1.route_ipv6_list, - c->c1.tuntap, ROUTE_OPTION_FLAGS(&c->options), - c->c2.es, &c->net_ctx); - } - /* actually close tun/tap device based on --down-pre flag */ - if (!c->options.down_pre) - { - do_close_tun_simple(c); - } - - /* Run the down script -- note that it will run at reduced - * privilege if, for example, "--user nobody" was used. */ - run_up_down(c->options.down_script, + /* delete any routes we added */ + if (c->c1.route_list || c->c1.route_ipv6_list) + { + run_up_down(c->options.route_predown_script, c->plugins, - OPENVPN_PLUGIN_DOWN, + OPENVPN_PLUGIN_ROUTE_PREDOWN, tuntap_actual, #ifdef _WIN32 adapter_index, @@ -1994,59 +1967,88 @@ do_close_tun(struct context *c, bool force) "init", signal_description(c->sig->signal_received, c->sig->signal_text), - "down", + "route-pre-down", c->c2.es); + delete_routes(c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, ROUTE_OPTION_FLAGS(&c->options), + c->c2.es, &c->net_ctx); + } + + /* actually close tun/tap device based on --down-pre flag */ + if (!c->options.down_pre) + { + do_close_tun_simple(c); + } + + /* Run the down script -- note that it will run at reduced + * privilege if, for example, "--user nobody" was used. */ + run_up_down(c->options.down_script, + c->plugins, + OPENVPN_PLUGIN_DOWN, + tuntap_actual, +#ifdef _WIN32 + adapter_index, +#endif + NULL, + c->c2.frame.tun_mtu, + print_in_addr_t(local, IA_EMPTY_IF_UNDEF, &gc), + print_in_addr_t(remote_netmask, IA_EMPTY_IF_UNDEF, &gc), + "init", + signal_description(c->sig->signal_received, + c->sig->signal_text), + "down", + c->c2.es); + #if defined(_WIN32) - if (c->options.block_outside_dns) + if (c->options.block_outside_dns) + { + if (!win_wfp_uninit(adapter_index, c->options.msg_channel)) { - if (!win_wfp_uninit(adapter_index, c->options.msg_channel)) - { - msg(M_FATAL, "Uninitialising WFP failed!"); - } + msg(M_FATAL, "Uninitialising WFP failed!"); } + } #endif - /* actually close tun/tap device based on --down-pre flag */ - if (c->options.down_pre) - { - do_close_tun_simple(c); - } + /* actually close tun/tap device based on --down-pre flag */ + if (c->options.down_pre) + { + do_close_tun_simple(c); } - else + } + else + { + /* run the down script on this restart if --up-restart was specified */ + if (c->options.up_restart) { - /* run the down script on this restart if --up-restart was specified */ - if (c->options.up_restart) - { - run_up_down(c->options.down_script, - c->plugins, - OPENVPN_PLUGIN_DOWN, - tuntap_actual, + run_up_down(c->options.down_script, + c->plugins, + OPENVPN_PLUGIN_DOWN, + tuntap_actual, #ifdef _WIN32 - adapter_index, + adapter_index, #endif - NULL, - c->c2.frame.tun_mtu, - print_in_addr_t(local, IA_EMPTY_IF_UNDEF, &gc), - print_in_addr_t(remote_netmask, IA_EMPTY_IF_UNDEF, &gc), - "restart", - signal_description(c->sig->signal_received, - c->sig->signal_text), - "down", - c->c2.es); - } + NULL, + c->c2.frame.tun_mtu, + print_in_addr_t(local, IA_EMPTY_IF_UNDEF, &gc), + print_in_addr_t(remote_netmask, IA_EMPTY_IF_UNDEF, &gc), + "restart", + signal_description(c->sig->signal_received, + c->sig->signal_text), + "down", + c->c2.es); + } #if defined(_WIN32) - if (c->options.block_outside_dns) + if (c->options.block_outside_dns) + { + if (!win_wfp_uninit(adapter_index, c->options.msg_channel)) { - if (!win_wfp_uninit(adapter_index, c->options.msg_channel)) - { - msg(M_FATAL, "Uninitialising WFP failed!"); - } + msg(M_FATAL, "Uninitialising WFP failed!"); } + } #endif - } } gc_free(&gc); }