Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2177?usp=email
to review the following change.
Change subject: networking_sitnl: Reduce boilerplate code for parameter checks
......................................................................
networking_sitnl: Reduce boilerplate code for parameter checks
Also removes some inconsistencies and some
sign conversion warnings.
Change-Id: I6f5ca53d4288b0921610f7a3f84ad55d2ab49118
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/networking_sitnl.c
1 file changed, 79 insertions(+), 137 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/77/2177/1
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 501c635..d33afc3 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -673,25 +673,64 @@
#ifdef ENABLE_SITNL
+static int
+get_ifindex(const char *func, const char *iface)
+{
+ if (!iface)
+ {
+ msg(M_WARN, "%s: passed NULL interface", func);
+ return -EINVAL;
+ }
+
+ const int ifindex = (int)if_nametoindex(iface);
+ if (ifindex == 0)
+ {
+ msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", func,
iface);
+ return -ENOENT;
+ }
+ return ifindex;
+}
+
+static int
+check_sitnl_addr_args(const char *func, sa_family_t af_family, const char
*iface, uint32_t *ifindex)
+{
+ switch (af_family)
+ {
+ case AF_INET:
+ case AF_INET6:
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ if (!iface)
+ {
+ msg(M_WARN, "%s: passed NULL interface", func);
+ return -EINVAL;
+ }
+
+ *ifindex = if_nametoindex(iface);
+ if (*ifindex == 0)
+ {
+ msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", func,
iface);
+ return -ENOENT;
+ }
+ return 0;
+}
+
+
int
net_iface_up(const openvpn_net_ctx_t *ctx, const char *iface, bool up)
{
struct sitnl_link_req req;
- int ifindex;
CLEAR(req);
- if (!iface)
+ const int ifindex = get_ifindex(__func__, iface);
+ if (ifindex < 0)
{
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN, "%s: rtnl: cannot get ifindex for %s: %s", __func__,
iface, strerror(errno));
- return -ENOENT;
+ return ifindex;
}
req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
@@ -719,14 +758,13 @@
net_iface_mtu_set(const openvpn_net_ctx_t *ctx, const char *iface, uint32_t
mtu)
{
struct sitnl_link_req req;
- int ifindex, ret = -1;
+ int ret = -1;
CLEAR(req);
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ const int ifindex = get_ifindex(__func__, iface);
+ if (ifindex < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
return -1;
}
@@ -750,14 +788,13 @@
net_addr_ll_set(const openvpn_net_ctx_t *ctx, const openvpn_net_iface_t
*iface, uint8_t *addr)
{
struct sitnl_link_req req;
- int ifindex, ret = -1;
+ int ret = -1;
CLEAR(req);
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ const int ifindex = get_ifindex(__func__, iface);
+ if (ifindex < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
return -1;
}
@@ -778,12 +815,19 @@
}
static int
-sitnl_addr_set(uint16_t cmd, uint16_t flags, int ifindex, sa_family_t
af_family,
+sitnl_addr_set(uint16_t cmd, uint16_t flags, const char *iface, sa_family_t
af_family,
const inet_address_t *local, const inet_address_t *remote, int
prefixlen)
{
struct sitnl_addr_req req;
- uint32_t size;
- int ret = -EINVAL;
+ size_t size;
+ uint32_t ifindex;
+
+ int ret = check_sitnl_addr_args(__func__, af_family, iface, &ifindex);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ ret = -EINVAL; /* for SITNL_ADDATTR */
CLEAR(req);
@@ -813,7 +857,7 @@
/* if no prefixlen has been specified, assume host address */
if (prefixlen == 0)
{
- prefixlen = size * 8;
+ prefixlen = (int)size * 8;
}
ASSERT(prefixlen <= UINT8_MAX);
req.i.ifa_prefixlen = (uint8_t)prefixlen;
@@ -848,64 +892,14 @@
sitnl_addr_ptp_add(sa_family_t af_family, const char *iface, const
inet_address_t *local,
const inet_address_t *remote)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN, "%s: cannot get ifindex for %s: %s", __func__, np(iface),
strerror(errno));
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
af_family, local,
+ return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, iface,
af_family, local,
remote, 0);
}
static int
sitnl_addr_ptp_del(sa_family_t af_family, const char *iface, const
inet_address_t *local)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, local, NULL, 0);
+ return sitnl_addr_set(RTM_DELADDR, 0, iface, af_family, local, NULL, 0);
}
static int
@@ -982,64 +976,14 @@
static int
sitnl_addr_add(sa_family_t af_family, const char *iface, const inet_address_t
*addr, int prefixlen)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
af_family, addr, NULL,
+ return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, iface,
af_family, addr, NULL,
prefixlen);
}
static int
sitnl_addr_del(sa_family_t af_family, const char *iface, const inet_address_t
*addr, int prefixlen)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, addr, NULL,
prefixlen);
+ return sitnl_addr_set(RTM_DELADDR, 0, iface, af_family, addr, NULL,
prefixlen);
}
int
@@ -1178,11 +1122,10 @@
if (iface)
{
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ ifindex = get_ifindex(__func__, iface);
+ if (ifindex < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
__func__, iface);
- return -ENOENT;
+ return ifindex;
}
}
@@ -1262,11 +1205,10 @@
if (iface)
{
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ ifindex = get_ifindex(__func__, iface);
+ if (ifindex < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
__func__, iface);
- return -ENOENT;
+ return ifindex;
}
}
@@ -1439,7 +1381,7 @@
net_iface_type(const openvpn_net_ctx_t *ctx, const char *iface, char
type[IFACE_TYPE_LEN_MAX])
{
struct sitnl_link_req req = {};
- int ifindex = if_nametoindex(iface);
+ int ifindex = (int)if_nametoindex(iface);
if (!ifindex)
{
@@ -1471,7 +1413,7 @@
net_iface_del(const openvpn_net_ctx_t *ctx, const char *iface)
{
struct sitnl_link_req req = {};
- int ifindex = if_nametoindex(iface);
+ int ifindex = (int)if_nametoindex(iface);
if (!ifindex)
{
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2177?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6f5ca53d4288b0921610f7a3f84ad55d2ab49118
Gerrit-Change-Number: 2177
Gerrit-PatchSet: 1
Gerrit-Owner: Frank Lichtenheld <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
