[Openvpn-devel,17/25] tun: extract close_tun_handle into its own fucntion and print correct type

Message ID 20220624083809.23487-18-a@unstable.cc
State Accepted
Headers show
Series ovpn-dco: introduce data-channel offload support | expand

Commit Message

Antonio Quartulli June 24, 2022, 8:38 a.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

This moves closing the tun handle into its own function and also prints
the adapter type we are operating on, instead hardcoding it to
tap-windows.

While at it, set the handle to NULL after closing, to prevent a double
close due to multiple invocations of this helper.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/tun.c | 76 ++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

Comments

Frank Lichtenheld July 4, 2022, 8:47 a.m. UTC | #1
On Fri, Jun 24, 2022 at 10:38:01AM +0200, Antonio Quartulli wrote:
> From: Arne Schwabe <arne@rfc2549.org>
> 
> This moves closing the tun handle into its own function and also prints
> the adapter type we are operating on, instead hardcoding it to
> tap-windows.
> 
> While at it, set the handle to NULL after closing, to prevent a double
> close due to multiple invocations of this helper.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  src/openvpn/tun.c | 76 ++++++++++++++++++++++++++---------------------
>  1 file changed, 42 insertions(+), 34 deletions(-)

Trivial enough. Also can be applied independently of all changes
before it in the series!

Acked-By: Frank Lichtenheld <frank@lichtenheld.com>

Regards,
Gert Doering July 19, 2022, 8:13 p.m. UTC | #2
Code change looks good.  Did not actually test anything, but it *does*
pass compilation on MinGW / Unbuntu 18.04 :-)

The change to print the actual adapter type confuses "--color-move=zebra", 
but is a nice enhancement.

Your patch has been applied to the master branch.

commit abceec65daed9aa6359314a0243246acc1cc25bf
Author: Arne Schwabe
Date:   Fri Jun 24 10:38:01 2022 +0200

     tun: extract close_tun_handle into its own fucntion and print correct type

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20220624083809.23487-18-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24527.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 5eefb0c6..f324ac91 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6767,6 +6767,47 @@  netsh_delete_address_dns(const struct tuntap *tt, bool ipv6, struct gc_arena *gc
     argv_free(&argv);
 }
 
+static void
+close_tun_handle(struct tuntap *tt)
+{
+    const char *adaptertype = print_windows_driver(tt->windows_driver);
+
+    if (tt->hand)
+    {
+        dmsg(D_WIN32_IO_LOW, "Attempting CancelIO on %s adapter", adaptertype);
+        if (!CancelIo(tt->hand))
+        {
+            msg(M_WARN | M_ERRNO, "Warning: CancelIO failed on %s adapter", adaptertype);
+        }
+    }
+
+    dmsg(D_WIN32_IO_LOW, "Attempting close of overlapped read event on %s adapter", adaptertype);
+    overlapped_io_close(&tt->reads);
+
+    dmsg(D_WIN32_IO_LOW, "Attempting close of overlapped write event on %s adapter", adaptertype);
+    overlapped_io_close(&tt->writes);
+
+    if (tt->hand)
+    {
+        dmsg(D_WIN32_IO_LOW, "Attempting CloseHandle on %s adapter", adaptertype);
+        if (!CloseHandle(tt->hand))
+        {
+            msg(M_WARN | M_ERRNO, "Warning: CloseHandle failed on %s adapter", adaptertype);
+        }
+        tt->hand = NULL;
+    }
+
+    if (tt->windows_driver == WINDOWS_DRIVER_WINTUN)
+    {
+        CloseHandle(tt->rw_handle.read);
+        CloseHandle(tt->rw_handle.write);
+        UnmapViewOfFile(tt->wintun_send_ring);
+        UnmapViewOfFile(tt->wintun_receive_ring);
+        CloseHandle(tt->wintun_send_ring_handle);
+        CloseHandle(tt->wintun_receive_ring_handle);
+    }
+}
+
 void
 close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
 {
@@ -6836,43 +6877,10 @@  close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
 
     dhcp_release(tt);
 
-    if (tt->hand != NULL)
-    {
-        dmsg(D_WIN32_IO_LOW, "Attempting CancelIO on TAP-Windows adapter");
-        if (!CancelIo(tt->hand))
-        {
-            msg(M_WARN | M_ERRNO, "Warning: CancelIO failed on TAP-Windows adapter");
-        }
-    }
-
-    dmsg(D_WIN32_IO_LOW, "Attempting close of overlapped read event on TAP-Windows adapter");
-    overlapped_io_close(&tt->reads);
-
-    dmsg(D_WIN32_IO_LOW, "Attempting close of overlapped write event on TAP-Windows adapter");
-    overlapped_io_close(&tt->writes);
-
-    if (tt->hand != NULL)
-    {
-        dmsg(D_WIN32_IO_LOW, "Attempting CloseHandle on TAP-Windows adapter");
-        if (!CloseHandle(tt->hand))
-        {
-            msg(M_WARN | M_ERRNO, "Warning: CloseHandle failed on TAP-Windows adapter");
-        }
-    }
+    close_tun_handle(tt);
 
     free(tt->actual_name);
 
-    if (tt->windows_driver == WINDOWS_DRIVER_WINTUN)
-    {
-        CloseHandle(tt->rw_handle.read);
-        CloseHandle(tt->rw_handle.write);
-        UnmapViewOfFile(tt->wintun_send_ring);
-        UnmapViewOfFile(tt->wintun_receive_ring);
-        CloseHandle(tt->wintun_send_ring_handle);
-        CloseHandle(tt->wintun_receive_ring_handle);
-    }
-
-
     clear_tuntap(tt);
     free(tt);
     gc_free(&gc);