| Message ID | 20191219223917.1614-1-simon@rozman.si |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel,1/7] tun.c: make Windows device lookup functions more general | expand |
Hi, Build in MSVC, tested and ran in debugger. The only nitpick is those methods (since you have touched them) +get_tap_by_guid(const char *guid, const struct tap_reg *tap_reg) +get_tap_by_name(const char *name, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg) could be renamed to "get_tuntap_by_*" or even better, "get_adapter_by_*" since they're adapter-type agnostic and deal with both tap and wintun. Acked-by: Lev Stipakov <lstipakov@gmail.com> pe 20. jouluk. 2019 klo 0.40 Simon Rozman (simon@rozman.si) kirjoitti: > Since the introduction of Wintun, not all network devices in Windows are > TAP-Windows6. Rather than returning a simple true/false answer, a couple > of functions were reworked to return a corresponding struct tap_reg * > or NULL instead. > > As it would make the code `tr = is_tap_win(...)` a bit awkward those > functions (both static) were renamed to better reflect their nature. > > Signed-off-by: Simon Rozman <simon@rozman.si> > --- > src/openvpn/tun.c | 39 ++++++++++++++++++++++----------------- > 1 file changed, 22 insertions(+), 17 deletions(-) > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index ad497a71..053a8232 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -3976,10 +3976,10 @@ show_tap_win_adapters(int msglev, int warnlev) > } > > /* > - * Confirm that GUID is a TAP-Windows adapter. > + * Lookup a TAP-Windows or Wintun adapter by GUID. > */ > -static bool > -is_tap_win(const char *guid, const struct tap_reg *tap_reg) > +static const struct tap_reg * > +get_tap_by_guid(const char *guid, const struct tap_reg *tap_reg) > { > const struct tap_reg *tr; > > @@ -3987,11 +3987,11 @@ is_tap_win(const char *guid, const struct tap_reg > *tap_reg) > { > if (guid && !strcmp(tr->guid, guid)) > { > - return true; > + return tr; > } > } > > - return false; > + return NULL; > } > > static const char * > @@ -4010,16 +4010,16 @@ guid_to_name(const char *guid, const struct > panel_reg *panel_reg) > return NULL; > } > > -static const char * > -name_to_guid(const char *name, const struct tap_reg *tap_reg, const > struct panel_reg *panel_reg) > +static const struct tap_reg * > +get_tap_by_name(const char *name, const struct tap_reg *tap_reg, const > struct panel_reg *panel_reg) > { > const struct panel_reg *pr; > > for (pr = panel_reg; pr != NULL; pr = pr->next) > { > - if (name && !strcmp(pr->name, name) && is_tap_win(pr->guid, > tap_reg)) > + if (name && !strcmp(pr->name, name)) > { > - return pr->guid; > + return get_tap_by_guid(pr->guid, tap_reg); > } > } > > @@ -4116,6 +4116,7 @@ get_device_guid(const char *name, > { > struct buffer ret = alloc_buf_gc(256, gc); > struct buffer actual = clear_buf(); > + const struct tap_reg *tr; > > /* Make sure we have at least one TAP adapter */ > if (!tap_reg) > @@ -4131,7 +4132,8 @@ get_device_guid(const char *name, > } > > /* Check if GUID was explicitly specified as --dev-node parameter */ > - if (is_tap_win(name, tap_reg)) > + tr = get_tap_by_guid(name, tap_reg); > + if (tr) > { > const char *act = guid_to_name(name, panel_reg); > buf_printf(&ret, "%s", name); > @@ -4148,11 +4150,11 @@ get_device_guid(const char *name, > > /* Lookup TAP adapter in network connections list */ > { > - const char *guid = name_to_guid(name, tap_reg, panel_reg); > - if (guid) > + tr = get_tap_by_name(name, tap_reg, panel_reg); > + if (tr) > { > buf_printf(&actual, "%s", name); > - buf_printf(&ret, "%s", guid); > + buf_printf(&ret, "%s", tr->guid); > return BSTR(&ret); > } > } > @@ -4696,11 +4698,14 @@ get_adapter_index_flexible(const char *name) /* > actual name or GUID */ > { > const struct tap_reg *tap_reg = get_tap_reg(&gc); > const struct panel_reg *panel_reg = get_panel_reg(&gc); > - const char *guid = name_to_guid(name, tap_reg, panel_reg); > - index = get_adapter_index_method_1(guid); > - if (index == TUN_ADAPTER_INDEX_INVALID) > + const struct tap_reg *tr = get_tap_by_name(name, tap_reg, > panel_reg); > + if (tr) > { > - index = get_adapter_index_method_2(guid); > + index = get_adapter_index_method_1(tr->guid); > + if (index == TUN_ADAPTER_INDEX_INVALID) > + { > + index = get_adapter_index_method_2(tr->guid); > + } > } > } > if (index == TUN_ADAPTER_INDEX_INVALID) > -- > 2.24.1.windows.2 > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel >
Hi, On Thu, Dec 19, 2019 at 11:39:11PM +0100, Simon Rozman wrote: > Since the introduction of Wintun, not all network devices in Windows are > TAP-Windows6. Rather than returning a simple true/false answer, a couple > of functions were reworked to return a corresponding struct tap_reg * > or NULL instead. > > As it would make the code `tr = is_tap_win(...)` a bit awkward those > functions (both static) were renamed to better reflect their nature. For the sake of the archives: Simon and Lev discussed these patches on IRC today, came up with proposed changes, and Simon will send a new "v2" series of all of them - so even if they have an ACK, I won't apply them just yet. Just to avoid confusion. gert
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index ad497a71..053a8232 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -3976,10 +3976,10 @@ show_tap_win_adapters(int msglev, int warnlev) } /* - * Confirm that GUID is a TAP-Windows adapter. + * Lookup a TAP-Windows or Wintun adapter by GUID. */ -static bool -is_tap_win(const char *guid, const struct tap_reg *tap_reg) +static const struct tap_reg * +get_tap_by_guid(const char *guid, const struct tap_reg *tap_reg) { const struct tap_reg *tr; @@ -3987,11 +3987,11 @@ is_tap_win(const char *guid, const struct tap_reg *tap_reg) { if (guid && !strcmp(tr->guid, guid)) { - return true; + return tr; } } - return false; + return NULL; } static const char * @@ -4010,16 +4010,16 @@ guid_to_name(const char *guid, const struct panel_reg *panel_reg) return NULL; } -static const char * -name_to_guid(const char *name, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg) +static const struct tap_reg * +get_tap_by_name(const char *name, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg) { const struct panel_reg *pr; for (pr = panel_reg; pr != NULL; pr = pr->next) { - if (name && !strcmp(pr->name, name) && is_tap_win(pr->guid, tap_reg)) + if (name && !strcmp(pr->name, name)) { - return pr->guid; + return get_tap_by_guid(pr->guid, tap_reg); } } @@ -4116,6 +4116,7 @@ get_device_guid(const char *name, { struct buffer ret = alloc_buf_gc(256, gc); struct buffer actual = clear_buf(); + const struct tap_reg *tr; /* Make sure we have at least one TAP adapter */ if (!tap_reg) @@ -4131,7 +4132,8 @@ get_device_guid(const char *name, } /* Check if GUID was explicitly specified as --dev-node parameter */ - if (is_tap_win(name, tap_reg)) + tr = get_tap_by_guid(name, tap_reg); + if (tr) { const char *act = guid_to_name(name, panel_reg); buf_printf(&ret, "%s", name); @@ -4148,11 +4150,11 @@ get_device_guid(const char *name, /* Lookup TAP adapter in network connections list */ { - const char *guid = name_to_guid(name, tap_reg, panel_reg); - if (guid) + tr = get_tap_by_name(name, tap_reg, panel_reg); + if (tr) { buf_printf(&actual, "%s", name); - buf_printf(&ret, "%s", guid); + buf_printf(&ret, "%s", tr->guid); return BSTR(&ret); } } @@ -4696,11 +4698,14 @@ get_adapter_index_flexible(const char *name) /* actual name or GUID */ { const struct tap_reg *tap_reg = get_tap_reg(&gc); const struct panel_reg *panel_reg = get_panel_reg(&gc); - const char *guid = name_to_guid(name, tap_reg, panel_reg); - index = get_adapter_index_method_1(guid); - if (index == TUN_ADAPTER_INDEX_INVALID) + const struct tap_reg *tr = get_tap_by_name(name, tap_reg, panel_reg); + if (tr) { - index = get_adapter_index_method_2(guid); + index = get_adapter_index_method_1(tr->guid); + if (index == TUN_ADAPTER_INDEX_INVALID) + { + index = get_adapter_index_method_2(tr->guid); + } } } if (index == TUN_ADAPTER_INDEX_INVALID)
Since the introduction of Wintun, not all network devices in Windows are TAP-Windows6. Rather than returning a simple true/false answer, a couple of functions were reworked to return a corresponding struct tap_reg * or NULL instead. As it would make the code `tr = is_tap_win(...)` a bit awkward those functions (both static) were renamed to better reflect their nature. Signed-off-by: Simon Rozman <simon@rozman.si> --- src/openvpn/tun.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-)