tun: remove tun_finalize()

tun_finalize() is essentially subset of socket_finalize() apart from:

 - using WSAFoo() functions instead of Foo()

 - "from" address is not returned

There is no clear official statement that one can use non-WSA
API on handles, so let's be on a safe side and use both.

Introduce sockethandle_t abstraction, which represents
socket and handle. Add SocketHandle* routines which call
proper API depends on underlying type in abstraction.

Rename socket_finalize() to sockethandle_finalize(), take
sockethandle_t and new routines into use and kick tun_finalize().

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20220117094917.178-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23555.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index f82386a1d9fd821ea2a0e71c3b0531540f144757..a905f993a85d4131659f35c3d318ba67a3b31a37 100644 (file)
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1115,7 +1115,8 @@ read_incoming_tun(struct context *c)
     }
     else
     {
-        read_tun_buffered(c->c1.tuntap, &c->c2.buf);
+        sockethandle_t sh = { .is_handle = true, .h = c->c1.tuntap->hand };
+        sockethandle_finalize(sh, &c->c1.tuntap->reads, &c->c2.buf, NULL);
     }
 #else  /* ifdef _WIN32 */
     ASSERT(buf_init(&c->c2.buf, FRAME_HEADROOM(&c->c2.frame)));

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index df7367469154b16944f783861de3ca369d521715..780c5cb3f9a1b05d31e52ee7d6a24e88d0871aac 100644 (file)
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -3198,7 +3198,8 @@ link_socket_read_tcp(struct link_socket *sock,
     if (!sock->stream_buf.residual_fully_formed)
     {
 #ifdef _WIN32
-        len = socket_finalize(sock->sd, &sock->reads, buf, NULL);
+        sockethandle_t sh = { .s = sock->sd };
+        len = sockethandle_finalize(sh, &sock->reads, buf, NULL);
 #else
         struct buffer frag;
         stream_buf_get_next(&sock->stream_buf, &frag);
@@ -3664,10 +3665,10 @@ socket_send_queue(struct link_socket *sock, struct buffer *buf, const struct lin
 }
 
 int
-socket_finalize(SOCKET s,
-                struct overlapped_io *io,
-                struct buffer *buf,
-                struct link_socket_actual *from)
+sockethandle_finalize(sockethandle_t sh,
+                      struct overlapped_io *io,
+                      struct buffer *buf,
+                      struct link_socket_actual *from)
 {
     int ret = -1;
     BOOL status;
@@ -3675,13 +3676,7 @@ socket_finalize(SOCKET s,
     switch (io->iostate)
     {
         case IOSTATE_QUEUED:
-            status = WSAGetOverlappedResult(
-                s,
-                &io->overlapped,
-                &io->size,
-                FALSE,
-                &io->flags
-                );
+            status = SocketHandleGetOverlappedResult(sh, io);
             if (status)
             {
                 /* successful return for a queued operation */
@@ -3693,18 +3688,18 @@ socket_finalize(SOCKET s,
                 io->iostate = IOSTATE_INITIAL;
                 ASSERT(ResetEvent(io->overlapped.hEvent));
 
-                dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion success [%d]", ret);
+                dmsg(D_WIN32_IO, "WIN32 I/O: Completion success [%d]", ret);
             }
             else
             {
                 /* error during a queued operation */
                 ret = -1;
-                if (WSAGetLastError() != WSA_IO_INCOMPLETE)
+                if (SocketHandleGetLastError(sh) != ERROR_IO_INCOMPLETE)
                 {
                     /* if no error (i.e. just not finished yet), then DON'T execute this code */
                     io->iostate = IOSTATE_INITIAL;
                     ASSERT(ResetEvent(io->overlapped.hEvent));
-                    msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion error");
+                    msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion error");
                 }
             }
             break;
@@ -3715,9 +3710,9 @@ socket_finalize(SOCKET s,
             if (io->status)
             {
                 /* error return for a non-queued operation */
-                WSASetLastError(io->status);
+                SocketHandleSetLastError(sh, io->status);
                 ret = -1;
-                msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion non-queued error");
+                msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion non-queued error");
             }
             else
             {
@@ -3727,14 +3722,14 @@ socket_finalize(SOCKET s,
                     *buf = io->buf;
                 }
                 ret = io->size;
-                dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion non-queued success [%d]", ret);
+                dmsg(D_WIN32_IO, "WIN32 I/O: Completion non-queued success [%d]", ret);
             }
             break;
 
         case IOSTATE_INITIAL: /* were we called without proper queueing? */
-            WSASetLastError(WSAEINVAL);
+            SocketHandleSetInvalError(sh);
             ret = -1;
-            dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion BAD STATE");
+            dmsg(D_WIN32_IO, "WIN32 I/O: Completion BAD STATE");
             break;
 
         default:
@@ -3742,7 +3737,7 @@ socket_finalize(SOCKET s,
     }
 
     /* return from address if requested */
-    if (from)
+    if (!sh.is_handle && from)
     {
         if (ret >= 0 && io->addr_defined)
         {

diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index cc1e0c3667a58412d5f22430ba39bbd2685f39ab..77c3d7209f403a6c6de0611ee7c4f7d2ae1f32cd 100644 (file)
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -262,11 +262,44 @@ int socket_send_queue(struct link_socket *sock,
                       struct buffer *buf,
                       const struct link_socket_actual *to);
 
-int socket_finalize(
-    SOCKET s,
-    struct overlapped_io *io,
-    struct buffer *buf,
-    struct link_socket_actual *from);
+typedef struct {
+    union {
+        SOCKET s;
+        HANDLE h;
+    };
+    bool is_handle;
+} sockethandle_t;
+
+int sockethandle_finalize(sockethandle_t sh,
+                          struct overlapped_io *io,
+                          struct buffer *buf,
+                          struct link_socket_actual *from);
+
+static inline BOOL
+SocketHandleGetOverlappedResult(sockethandle_t sh, struct overlapped_io *io)
+{
+    return sh.is_handle ?
+        GetOverlappedResult(sh.h, &io->overlapped, &io->size, FALSE) :
+        WSAGetOverlappedResult(sh.s, &io->overlapped, &io->size, FALSE, &io->flags);
+}
+
+static inline int
+SocketHandleGetLastError(sockethandle_t sh)
+{
+    return sh.is_handle ? (int)GetLastError() : WSAGetLastError();
+}
+
+inline static void
+SocketHandleSetLastError(sockethandle_t sh, DWORD err)
+{
+    sh.is_handle ? SetLastError(err) : WSASetLastError(err);
+}
+
+static inline void
+SocketHandleSetInvalError(sockethandle_t sh)
+{
+    sh.is_handle ? SetLastError(ERROR_INVALID_FUNCTION) : WSASetLastError(WSAEINVAL);
+}
 
 #else  /* ifdef _WIN32 */
 
@@ -1020,7 +1053,8 @@ link_socket_read_udp_win32(struct link_socket *sock,
                            struct buffer *buf,
                            struct link_socket_actual *from)
 {
-    return socket_finalize(sock->sd, &sock->reads, buf, from);
+    sockethandle_t sh = { .s = sock->sd };
+    return sockethandle_finalize(sh, &sock->reads, buf, from);
 }
 
 #else  /* ifdef _WIN32 */
@@ -1078,9 +1112,10 @@ link_socket_write_win32(struct link_socket *sock,
 {
     int err = 0;
     int status = 0;
+    sockethandle_t sh = { .s = sock->sd };
     if (overlapped_io_active(&sock->writes))
     {
-        status = socket_finalize(sock->sd, &sock->writes, NULL, NULL);
+        status = sockethandle_finalize(sh, &sock->writes, NULL, NULL);
         if (status < 0)
         {
             err = WSAGetLastError();

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 12bdd20056274b23f5d43eb8c5f82334b8bc1efe..fe32127b6bcb6a98aae91161f785a31edcbf17a4 100644 (file)
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -3561,87 +3561,29 @@ tun_write_queue(struct tuntap *tt, struct buffer *buf)
 }
 
 int
-tun_finalize(
-    HANDLE h,
-    struct overlapped_io *io,
-    struct buffer *buf)
+tun_write_win32(struct tuntap *tt, struct buffer *buf)
 {
-    int ret = -1;
-    BOOL status;
-
-    switch (io->iostate)
+    int err = 0;
+    int status = 0;
+    if (overlapped_io_active(&tt->writes))
     {
-        case IOSTATE_QUEUED:
-            status = GetOverlappedResult(
-                h,
-                &io->overlapped,
-                &io->size,
-                FALSE
-                );
-            if (status)
-            {
-                /* successful return for a queued operation */
-                if (buf)
-                {
-                    *buf = io->buf;
-                }
-                ret = io->size;
-                io->iostate = IOSTATE_INITIAL;
-                ASSERT(ResetEvent(io->overlapped.hEvent));
-                dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion success [%d]", ret);
-            }
-            else
-            {
-                /* error during a queued operation */
-                ret = -1;
-                if (GetLastError() != ERROR_IO_INCOMPLETE)
-                {
-                    /* if no error (i.e. just not finished yet),
-                     * then DON'T execute this code */
-                    io->iostate = IOSTATE_INITIAL;
-                    ASSERT(ResetEvent(io->overlapped.hEvent));
-                    msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: TAP Completion error");
-                }
-            }
-            break;
-
-        case IOSTATE_IMMEDIATE_RETURN:
-            io->iostate = IOSTATE_INITIAL;
-            ASSERT(ResetEvent(io->overlapped.hEvent));
-            if (io->status)
-            {
-                /* error return for a non-queued operation */
-                SetLastError(io->status);
-                ret = -1;
-                msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: TAP Completion non-queued error");
-            }
-            else
-            {
-                /* successful return for a non-queued operation */
-                if (buf)
-                {
-                    *buf = io->buf;
-                }
-                ret = io->size;
-                dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion non-queued success [%d]", ret);
-            }
-            break;
-
-        case IOSTATE_INITIAL: /* were we called without proper queueing? */
-            SetLastError(ERROR_INVALID_FUNCTION);
-            ret = -1;
-            dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion BAD STATE");
-            break;
-
-        default:
-            ASSERT(0);
+        sockethandle_t sh = { .is_handle = true, .h = tt->hand };
+        status = sockethandle_finalize(sh, &tt->writes, NULL, NULL);
+        if (status < 0)
+        {
+            err = GetLastError();
+        }
     }
-
-    if (buf)
+    tun_write_queue(tt, buf);
+    if (status < 0)
     {
-        buf->len = ret;
+        SetLastError(err);
+        return status;
+    }
+    else
+    {
+        return BLEN(buf);
     }
-    return ret;
 }
 
 static const struct device_instance_id_interface *

diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index d4657537c69fccda173209da3f3658ddcfa05f34..739e93d7615aec11fcec4151de94c97e9b60de6a 100644 (file)
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -437,8 +437,6 @@ int tun_read_queue(struct tuntap *tt, int maxsize);
 
 int tun_write_queue(struct tuntap *tt, struct buffer *buf);
 
-int tun_finalize(HANDLE h, struct overlapped_io *io, struct buffer *buf);
-
 static inline bool
 tuntap_stop(int status)
 {
@@ -466,36 +464,7 @@ tuntap_abort(int status)
     return false;
 }
 
-static inline int
-tun_write_win32(struct tuntap *tt, struct buffer *buf)
-{
-    int err = 0;
-    int status = 0;
-    if (overlapped_io_active(&tt->writes))
-    {
-        status = tun_finalize(tt->hand, &tt->writes, NULL);
-        if (status < 0)
-        {
-            err = GetLastError();
-        }
-    }
-    tun_write_queue(tt, buf);
-    if (status < 0)
-    {
-        SetLastError(err);
-        return status;
-    }
-    else
-    {
-        return BLEN(buf);
-    }
-}
-
-static inline int
-read_tun_buffered(struct tuntap *tt, struct buffer *buf)
-{
-    return tun_finalize(tt->hand, &tt->reads, buf);
-}
+int tun_write_win32(struct tuntap *tt, struct buffer *buf);
 
 static inline ULONG
 wintun_ring_packet_align(ULONG size)