[Openvpn-devel,M] Change in openvpn[master]: dco-win: factor out getting dco version info own function

Message ID d7eebe957844eb6ab095927ec64308e4eb193a95-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: dco-win: factor out getting dco version info own function | expand

Commit Message

plaisthos (Code Review) Sept. 6, 2024, 7:43 a.m. UTC
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/733?usp=email

to review the following change.


Change subject: dco-win: factor out getting dco version info own function
......................................................................

dco-win: factor out getting dco version info own function

Instead of having duplicated code to get dco version
for data_v3 checks and for version info, have it in
one place and store the version in dco context - the
latter is a preparation for dco v2 support (multi-peer).

Change-Id: I8e8ddd35bd3cc3334faf7f57118d1892512ae9f7
Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
M src/openvpn/dco_win.c
M src/openvpn/dco_win.h
2 files changed, 51 insertions(+), 56 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/33/733/1

Patch

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 189864b..cf825b3 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -52,11 +52,54 @@ 
     return tt;
 }
 
+static void
+dco_get_version(OVPN_VERSION *version)
+{
+    HANDLE h = CreateFile("\\\\.\\ovpn-dco-ver", GENERIC_READ,
+                          0, NULL, OPEN_EXISTING, 0, NULL);
+
+    if (h == INVALID_HANDLE_VALUE)
+    {
+        /* fallback to a "normal" device, this will fail if device is already in use */
+        h = CreateFile("\\\\.\\ovpn-dco", GENERIC_READ,
+                       0, NULL, OPEN_EXISTING, 0, NULL);
+    }
+
+    if (h == INVALID_HANDLE_VALUE)
+    {
+        goto done;
+    }
+
+    DWORD bytes_returned = 0;
+    if (!DeviceIoControl(h, OVPN_IOCTL_GET_VERSION, NULL, 0,
+                         version, sizeof(*version), &bytes_returned, NULL))
+    {
+        goto done;
+    }
+
+done:
+    if (h != INVALID_HANDLE_VALUE)
+    {
+        CloseHandle(h);
+    }
+
+    msg(D_DCO_DEBUG, "dco version: %ld.%ld.%ld", version->Major, version->Minor, version->Patch);
+}
+
+static inline
+bool
+dco_version_supports_data_v3(OVPN_VERSION *version)
+{
+    return (version->Major > 1) || (version->Minor >= 4);
+}
+
 bool
 ovpn_dco_init(int mode, dco_context_t *dco)
 {
-    dco->supports_data_v3 = dco_supports_data_v3(NULL);
-    msg(D_DCO_DEBUG, "dco supports data_v3: %d", dco->supports_data_v3);
+    dco_get_version(&dco->version);
+    dco->supports_data_v3 = dco_version_supports_data_v3(&dco->version);
+
+    msg(D_DCO_DEBUG, "dco data_v3: %d", dco->supports_data_v3);
 
     return true;
 }
@@ -428,33 +471,9 @@ 
 dco_version_string(struct gc_arena *gc)
 {
     OVPN_VERSION version;
-    ZeroMemory(&version, sizeof(OVPN_VERSION));
+    ZeroMemory(&version, sizeof(version));
 
-    /* first, try a non-exclusive control device, available from 1.3.0 */
-    HANDLE h = CreateFile("\\\\.\\ovpn-dco-ver", GENERIC_READ,
-                          0, NULL, OPEN_EXISTING, 0, NULL);
-
-    if (h == INVALID_HANDLE_VALUE)
-    {
-        /* fallback to a "normal" device, this will fail if device is already in use */
-        h = CreateFile("\\\\.\\ovpn-dco", GENERIC_READ,
-                       0, NULL, OPEN_EXISTING, 0, NULL);
-    }
-
-    if (h == INVALID_HANDLE_VALUE)
-    {
-        return "N/A";
-    }
-
-    DWORD bytes_returned = 0;
-    if (!DeviceIoControl(h, OVPN_IOCTL_GET_VERSION, NULL, 0,
-                         &version, sizeof(version), &bytes_returned, NULL))
-    {
-        CloseHandle(h);
-        return "N/A";
-    }
-
-    CloseHandle(h);
+    dco_get_version(&version);
 
     struct buffer out = alloc_buf_gc(256, gc);
     buf_printf(&out, "%ld.%ld.%ld", version.Major, version.Minor, version.Patch);
@@ -538,36 +557,11 @@ 
 bool
 dco_supports_data_v3(struct context *c)
 {
-    bool res = false;
-
-    HANDLE h = CreateFile("\\\\.\\ovpn-dco-ver", GENERIC_READ,
-                          0, NULL, OPEN_EXISTING, 0, NULL);
-
-    if (h == INVALID_HANDLE_VALUE)
-    {
-        goto done;
-    }
-
     OVPN_VERSION version;
-    ZeroMemory(&version, sizeof(OVPN_VERSION));
+    ZeroMemory(&version, sizeof(version));
+    dco_get_version(&version);
 
-    DWORD bytes_returned = 0;
-    if (!DeviceIoControl(h, OVPN_IOCTL_GET_VERSION, NULL, 0,
-                         &version, sizeof(version), &bytes_returned, NULL))
-    {
-        goto done;
-    }
-
-    /* data_v3 is supported starting from 1.4 */
-    res = (version.Major > 1) || (version.Minor >= 4);
-
-done:
-    if (h != INVALID_HANDLE_VALUE)
-    {
-        CloseHandle(h);
-    }
-
-    return res;
+    return dco_version_supports_data_v3(&version);
 }
 
 #endif /* defined(_WIN32) */
diff --git a/src/openvpn/dco_win.h b/src/openvpn/dco_win.h
index f58488f..ce57817 100644
--- a/src/openvpn/dco_win.h
+++ b/src/openvpn/dco_win.h
@@ -34,6 +34,7 @@ 
 struct dco_context {
     struct tuntap *tt;
     bool supports_data_v3;
+    OVPN_VERSION version;
 };
 
 typedef struct dco_context dco_context_t;