Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1519?usp=email
to review the following change.
Change subject: Win32: Make interface index DWORD consistently
......................................................................
Win32: Make interface index DWORD consistently
Previously we had a weird mix of int and DWORD. But the
Win32 APIs seem to be consistent (they have different names,
but NET_IFINDEX is ULONG is DWORD). So use that.
Change-Id: I38bb2d1fa66c543e8bcf47b7d77b6154d1895f81
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M include/openvpn-msg.h
M src/openvpn/dns.c
M src/openvpn/route.c
M src/openvpn/tun.c
M src/openvpn/tun.h
M src/openvpn/win32.c
M src/openvpnserv/interactive.c
7 files changed, 48 insertions(+), 57 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1519/1
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a0cec53..34f50c8 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -66,7 +66,8 @@
typedef struct
{
- int index;
+#define TUN_ADAPTER_INDEX_INVALID ((DWORD)-1)
+ DWORD index;
char name[256];
} interface_t;
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index fd388ce..2137a75 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -468,9 +468,10 @@
make_domain_list("dns search domains", search_domains, false,
nrpt.search_domains,
sizeof(nrpt.search_domains));
- msg(D_LOW, "%s NRPT DNS%s%s on '%s' (if_index = %d) using service",
+ msg(D_LOW, "%s NRPT DNS%s%s on '%s' (if_index = %lu) using service",
(add ? "Setting" : "Deleting"), nrpt.resolve_domains[0] != 0 ? ",
resolve domains" : "",
- nrpt.search_domains[0] != 0 ? ", search domains" : "",
nrpt.iface.name, nrpt.iface.index);
+ nrpt.search_domains[0] != 0 ? ", search domains" : "",
+ nrpt.iface.name, nrpt.iface.index);
send_msg_iservice(o->msg_channel, &nrpt, sizeof(nrpt), &ack, "DNS");
}
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index b196713..af21b92 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2470,8 +2470,8 @@
}
}
- msg(D_ROUTE, "TEST ROUTES: %d/%d succeeded len=%d ret=%d a=%d u/d=%s",
good, count, len,
- (int)ret, ambig, adapter_up ? "up" : "down");
+ msg(D_ROUTE, "TEST ROUTES: %d/%d succeeded len=%d ret=%u a=%d u/d=%s",
+ good, count, len, ret, ambig, adapter_up ? "up" : "down");
gc_free(&gc);
return ret;
@@ -2495,9 +2495,9 @@
const DWORD index = row->dwForwardIfIndex;
const DWORD metric = row->dwForwardMetric1;
- dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%d m=%d", i,
+ dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%lu m=%lu", i,
print_in_addr_t((in_addr_t)net, 0, &gc),
print_in_addr_t((in_addr_t)mask, 0, &gc),
- (int)index, (int)metric);
+ index, metric);
if (!net && !mask && metric < lowest_metric)
{
@@ -2508,7 +2508,7 @@
}
}
- dmsg(D_ROUTE_DEBUG, "GDGR: best=%d lm=%u", best, (unsigned
int)lowest_metric);
+ dmsg(D_ROUTE_DEBUG, "GDGR: best=%d lm=%lu", best, lowest_metric);
gc_free(&gc);
return ret;
@@ -2542,7 +2542,7 @@
goto done;
}
- msg(D_ROUTE_DEBUG, "GetBestInterfaceEx() returned if=%d",
(int)best_if_index);
+ msg(D_ROUTE_DEBUG, "GetBestInterfaceEx() returned if=%lu", best_if_index);
/* get the routing information (such as NextHop) for the destination and
interface */
NET_LUID luid;
@@ -2552,8 +2552,8 @@
status = GetBestRoute2(&luid, best_if_index, NULL, dest, 0, best_route,
&best_src);
if (status != NO_ERROR)
{
- msg(D_ROUTE, "NOTE: GetIpForwardEntry2 returned error: %s (code=%u)",
- strerror_win32(status, gc), (unsigned int)status);
+ msg(D_ROUTE, "NOTE: GetIpForwardEntry2 returned error: %s (code=%lu)",
+ strerror_win32(status, gc), status);
goto done;
}
@@ -2647,8 +2647,8 @@
print_in_addr_t(r->gateway, 0, &gc), count);
}
- dmsg(D_ROUTE_DEBUG, "DEBUG: route find if: on_tun=%d count=%d index=%d",
on_tun, count,
- (int)ret);
+ dmsg(D_ROUTE_DEBUG, "DEBUG: route find if: on_tun=%d count=%d index=%lu",
+ on_tun, count, ret);
gc_free(&gc);
return ret;
@@ -2687,8 +2687,8 @@
print_in6_addr(BestRoute.DestinationPrefix.Prefix.Ipv6.sin6_addr, 0,
&gc),
BestRoute.DestinationPrefix.PrefixLength,
print_in6_addr(BestRoute.NextHop.Ipv6.sin6_addr, 0, &gc));
- msg(D_ROUTE, "GDG6: Metric=%d, Loopback=%d, AA=%d, I=%d",
(int)BestRoute.Metric,
- (int)BestRoute.Loopback, (int)BestRoute.AutoconfigureAddress,
(int)BestRoute.Immortal);
+ msg(D_ROUTE, "GDG6: Metric=%lu, Loopback=%u, AA=%u, I=%u",
BestRoute.Metric,
+ BestRoute.Loopback, BestRoute.AutoconfigureAddress,
BestRoute.Immortal);
rgi6->gateway.addr_ipv6 = BestRoute.NextHop.Ipv6.sin6_addr;
rgi6->adapter_index = BestRoute.InterfaceIndex;
@@ -2855,7 +2855,7 @@
ret = (ack.error_number == ERROR_OBJECT_ALREADY_EXISTS) ? RTA_EEXIST :
RTA_ERROR;
if (ret == RTA_ERROR)
{
- msg(M_WARN, "ERROR: route %s failed using service: %s [status=%u
if_index=%d]",
+ msg(M_WARN, "ERROR: route %s failed using service: %s [status=%u
if_index=%lu]",
(add ? "addition" : "deletion"),
strerror_win32(ack.error_number, &gc),
ack.error_number, rt->iface.index);
}
@@ -2869,17 +2869,12 @@
return ret;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-
/* Returns RTA_SUCCESS on success, RTA_EEXIST if route exists, RTA_ERROR on
error */
static int
do_route_ipv4_service(const bool add, const struct route_ipv4 *r, const struct
tuntap *tt)
{
DWORD if_index = windows_route_find_if_index(r, tt);
- if (if_index == ~0)
+ if (if_index == TUN_ADAPTER_INDEX_INVALID)
{
return RTA_ERROR;
}
@@ -3023,10 +3018,6 @@
return status;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
/* Returns RTA_SUCCESS on success, RTA_EEXIST if route exists, RTA_ERROR on
error */
static int
add_route_service(const struct route_ipv4 *r, const struct tuntap *tt)
@@ -3057,14 +3048,14 @@
format_route_entry(const MIB_IPFORWARDROW *r, struct gc_arena *gc)
{
struct buffer out = alloc_buf_gc(256, gc);
- buf_printf(&out, "%s %s %s p=%d i=%d t=%d pr=%d a=%d h=%d
m=%d/%d/%d/%d/%d",
+ buf_printf(&out, "%s %s %s p=%lu i=%lu t=%lu pr=%lu a=%lu h=%lu
m=%lu/%lu/%lu/%lu/%lu",
print_in_addr_t(r->dwForwardDest, IA_NET_ORDER, gc),
print_in_addr_t(r->dwForwardMask, IA_NET_ORDER, gc),
- print_in_addr_t(r->dwForwardNextHop, IA_NET_ORDER, gc),
(int)r->dwForwardPolicy,
- (int)r->dwForwardIfIndex, (int)r->dwForwardType,
(int)r->dwForwardProto,
- (int)r->dwForwardAge, (int)r->dwForwardNextHopAS,
(int)r->dwForwardMetric1,
- (int)r->dwForwardMetric2, (int)r->dwForwardMetric3,
(int)r->dwForwardMetric4,
- (int)r->dwForwardMetric5);
+ print_in_addr_t(r->dwForwardNextHop, IA_NET_ORDER, gc),
r->dwForwardPolicy,
+ r->dwForwardIfIndex, r->dwForwardType, r->dwForwardProto,
+ r->dwForwardAge, r->dwForwardNextHopAS, r->dwForwardMetric1,
+ r->dwForwardMetric2, r->dwForwardMetric3, r->dwForwardMetric4,
+ r->dwForwardMetric5);
return BSTR(&out);
}
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index ad77117..5c1ba25 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -158,7 +158,7 @@
if (ack.error_number != NO_ERROR)
{
- msg(M_WARN, "TUN: %s address failed using service: %s [status=%u
if_index=%d]",
+ msg(M_WARN, "TUN: %s address failed using service: %s [status=%u
if_index=%lu]",
(add ? "adding" : "deleting"), strerror_win32(ack.error_number,
&gc), ack.error_number,
addr.iface.index);
goto out;
@@ -220,7 +220,7 @@
strncpy(dns.domains + dstlen, o->domain_search_list[i], srclen + 1);
}
- msg(D_LOW, "%s DNS domains on '%s' (if_index = %d) using service",
+ msg(D_LOW, "%s DNS domains on '%s' (if_index = %lu) using service",
(add ? "Setting" : "Deleting"), dns.iface.name, dns.iface.index);
if (!send_msg_iservice(o->msg_channel, &dns, sizeof(dns), &ack, "TUN"))
{
@@ -289,7 +289,7 @@
}
}
- msg(D_LOW, "%s %s dns servers on '%s' (if_index = %d) using service",
+ msg(D_LOW, "%s %s dns servers on '%s' (if_index = %lu) using service",
(add ? "Setting" : "Deleting"), ip_proto_name, dns.iface.name,
dns.iface.index);
if (!send_msg_iservice(pipe, &dns, sizeof(dns), &ack, "TUN"))
@@ -346,7 +346,7 @@
wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
}
- msg(D_LOW, "%s WINS servers on '%s' (if_index = %d) using service",
+ msg(D_LOW, "%s WINS servers on '%s' (if_index = %lu) using service",
(add ? "Setting" : "Deleting"), wins.iface.name, wins.iface.index);
if (!send_msg_iservice(pipe, &wins, sizeof(wins), &ack, "TUN"))
@@ -398,13 +398,13 @@
if (ack.error_number != NO_ERROR)
{
- msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s
[status=%u if_index=%d]",
+ msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s
[status=%u if_index=%lu]",
family_name, strerror_win32(ack.error_number, &gc),
ack.error_number,
mtu_msg.iface.index);
}
else
{
- msg(M_INFO, "%s MTU set to %d on interface %d using service",
family_name, mtu,
+ msg(M_INFO, "%s MTU set to %d on interface %lu using service",
family_name, mtu,
mtu_msg.iface.index);
ret = true;
}
@@ -4456,8 +4456,8 @@
list = list->Next;
}
- dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d
metric=%d",
- print_in_addr_t(ip, 0, &gc), print_in_addr_t(highest_netmask, 0,
&gc), (int)ret,
+ dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%lu count=%d
metric=%d",
+ print_in_addr_t(ip, 0, &gc), print_in_addr_t(highest_netmask, 0,
&gc), ret,
count ? *count : -1, lowest_metric);
if (ret == TUN_ADAPTER_INDEX_INVALID && count)
@@ -4634,7 +4634,7 @@
show_adapter(msglvl_t msglevel, const IP_ADAPTER_INFO *a, struct gc_arena *gc)
{
msg(msglevel, "%s", a->Description);
- msg(msglevel, " Index = %d", (int)a->Index);
+ msg(msglevel, " Index = %lu", a->Index);
msg(msglevel, " GUID = %s", a->AdapterName);
msg(msglevel, " IP = %s", format_ip_addr_string(&a->IpAddressList, gc));
msg(msglevel, " MAC = %s", format_hex_ex(a->Address, a->AddressLength, 0,
1, ":", gc));
@@ -5215,12 +5215,12 @@
if (ack.error_number != NO_ERROR)
{
- msg(M_NONFATAL, "TUN: enabling dhcp using service failed: %s
[status=%u if_index=%d]",
+ msg(M_NONFATAL, "TUN: enabling dhcp using service failed: %s
[status=%u if_index=%lu]",
strerror_win32(ack.error_number, &gc), ack.error_number,
dhcp.iface.index);
}
else
{
- msg(M_INFO, "DHCP enabled on interface %d using service",
dhcp.iface.index);
+ msg(M_INFO, "DHCP enabled on interface %lu using service",
dhcp.iface.index);
ret = true;
}
@@ -5510,7 +5510,7 @@
/* flush arp cache */
if (tt->backend_driver == WINDOWS_DRIVER_TAP_WINDOWS6 && index !=
TUN_ADAPTER_INDEX_INVALID)
{
- DWORD status = -1;
+ DWORD status = (DWORD)-1;
if (tt->options.msg_channel)
{
@@ -5534,7 +5534,7 @@
{
msg(M_INFO, "Successful ARP Flush on interface [%lu] %s", index,
device_guid);
}
- else if (status != -1)
+ else if (status != (DWORD)-1)
{
msg(D_TUNTAP_INFO,
"NOTE: FlushIpNetTable failed on interface [%lu] %s
(status=%lu) : %s", index,
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index a226e37..388f5b7 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -61,8 +61,6 @@
#if defined(_WIN32) || defined(TARGET_ANDROID) || defined(DHCP_UNIT_TEST)
-#define TUN_ADAPTER_INDEX_INVALID ((DWORD)-1)
-
/* time constants for --ip-win32 adaptive */
#define IPW32_SET_ADAPTIVE_DELAY_WINDOW 300
#define IPW32_SET_ADAPTIVE_TRY_NETSH 20
@@ -230,7 +228,7 @@
in_addr_t adapter_netmask;
/* Windows adapter index for TAP-Windows adapter,
- * ~0 if undefined */
+ * TUN_ADAPTER_INDEX_INVALID if undefined */
DWORD adapter_index;
int standby_iter;
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index ac449fd..b2a7331 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1185,7 +1185,7 @@
if (ack.error_number != NO_ERROR)
{
msg(M_WARN,
- "WFP block: %s block filters using service failed: %s [status=0x%x
if_index=%d]",
+ "WFP block: %s block filters using service failed: %s [status=0x%x
if_index=%lu]",
(add ? "adding" : "deleting"), strerror_win32(ack.error_number,
&gc), ack.error_number,
data.iface.index);
goto out;
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 09c709e..b50aeb3 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -96,7 +96,7 @@
typedef struct
{
HANDLE engine;
- int index;
+ DWORD index;
int metric_v4;
int metric_v6;
} wfp_block_data_t;
@@ -584,7 +584,7 @@
addr_row->Address = sockaddr_inet(msg->family, &msg->address);
addr_row->OnLinkPrefixLength = (UINT8)msg->prefix_len;
- if (msg->iface.index != -1)
+ if (msg->iface.index != TUN_ADAPTER_INDEX_INVALID)
{
addr_row->InterfaceIndex = msg->iface.index;
}
@@ -667,7 +667,7 @@
fwd_row->DestinationPrefix.PrefixLength = (UINT8)msg->prefix_len;
fwd_row->NextHop = sockaddr_inet(msg->family, &msg->gateway);
- if (msg->iface.index != -1)
+ if (msg->iface.index != TUN_ADAPTER_INDEX_INVALID)
{
fwd_row->InterfaceIndex = msg->iface.index;
}
@@ -1010,7 +1010,7 @@
* if action = "set" then "static" is added before $addr
*/
static DWORD
-netsh_wins_cmd(const wchar_t *action, int if_index, const wchar_t *addr)
+netsh_wins_cmd(const wchar_t *action, DWORD if_index, const wchar_t *addr)
{
DWORD err = 0;
int timeout = 30000; /* in msec */
@@ -1036,7 +1036,7 @@
/* cmd template:
* netsh interface ip $action wins $if_name $static $addr
*/
- const wchar_t *fmt = L"netsh interface ip %ls wins %d %ls %ls";
+ const wchar_t *fmt = L"netsh interface ip %ls wins %lu %ls %ls";
/* max cmdline length in wchars -- include room for worst case and some */
size_t ncmdline = wcslen(fmt) + 11 /*if_index*/ + wcslen(action) +
wcslen(addr)
@@ -2984,7 +2984,7 @@
*/
}
- int *if_index = malloc(sizeof(msg->iface.index));
+ PDWORD if_index = malloc(sizeof(msg->iface.index));
if (if_index)
{
*if_index = msg->iface.index;
@@ -3017,7 +3017,7 @@
/* cmd template:
* netsh interface ipv4 set address name=$if_index source=dhcp
*/
- const wchar_t *fmt = L"netsh interface ipv4 set address name=\"%d\"
source=dhcp";
+ const wchar_t *fmt = L"netsh interface ipv4 set address name=\"%lu\"
source=dhcp";
/* max cmdline length in wchars -- include room for if index:
* 10 chars for 32 bit int in decimal and +1 for NUL
@@ -3250,7 +3250,7 @@
break;
case undo_wins:
- netsh_wins_cmd(L"delete", *(int *)item->data, NULL);
+ netsh_wins_cmd(L"delete", *(PDWORD)item->data, NULL);
break;
case wfp_block:
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1519?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I38bb2d1fa66c543e8bcf47b7d77b6154d1895f81
Gerrit-Change-Number: 1519
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
