Hi Benjamin,
On Tue, Sep 10, 2019 at 05:52:58PM +0200, Benjamin Cama wrote:
---
src/ipaddress.c | 6 +++++-
src/ipconfig.c | 18 +++++++++++++-----
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/ipaddress.c b/src/ipaddress.c
index d63d95c3..ebdabbb7 100644
--- a/src/ipaddress.c
+++ b/src/ipaddress.c
@@ -149,7 +149,11 @@ int connman_ipaddress_set_ipv4(struct connman_ipaddress *ipaddress,
ipaddress->family = AF_INET;
- ipaddress->prefixlen = connman_ipaddress_calc_netmask_len(netmask);
+ /* check if netmask or prefixlen */
+ if (g_strrstr(netmask, "."))
+ ipaddress->prefixlen = connman_ipaddress_calc_netmask_len(netmask);
+ else
+ ipaddress->prefixlen = g_ascii_strtoull(netmask, NULL, 10);
I think we should change connman_ipaddress_set_ipv4() to match
connman_ipaddress_set_ipv6() in regards of the argument
list. Basically, change the netmask argument to prefix len and only
pass that in. And create the netmask here again. That is slightly
suboptimal but I prefer that we keep the both function more
alike.
g_free(ipaddress->local);
ipaddress->local = g_strdup(address);
diff --git a/src/ipconfig.c b/src/ipconfig.c
index 25657733..b5ad1a36 100644
--- a/src/ipconfig.c
+++ b/src/ipconfig.c
@@ -1997,7 +1997,9 @@ int __connman_ipconfig_set_config(struct connman_ipconfig
*ipconfig,
dbus_message_iter_get_basic(&value, &prefix_length);
- if (prefix_length < 0 || prefix_length > 128)
+ if (ipconfig->type == CONNMAN_IPCONFIG_TYPE_IPV6 && (prefix_length < 0
|| prefix_length > 128))
+ return -EINVAL;
+ else if (ipconfig->type == CONNMAN_IPCONFIG_TYPE_IPV4 && (prefix_length
< 0 || prefix_length > 32))
Wrap the two lines. They are a bit too long.
return -EINVAL;
} else if (g_str_equal(key, "Netmask")) {
if (type != DBUS_TYPE_STRING)
@@ -2071,10 +2073,16 @@ int __connman_ipconfig_set_config(struct connman_ipconfig
*ipconfig,
ipconfig->method = method;
- if (ipconfig->type == CONNMAN_IPCONFIG_TYPE_IPV4)
- connman_ipaddress_set_ipv4(ipconfig->address,
- address, netmask, gateway);
- else
+ if (ipconfig->type == CONNMAN_IPCONFIG_TYPE_IPV4) {
+ if (prefix_length) {
+ char prefix_length_str[3];
+ sprintf(prefix_length_str, "%d", prefix_length);
calculate the prefix_length from netmaks above whenever the
prefix_length is not set. Then you don't need to handle special case
inse this if block.
BTW, isn't the buffer too short? The string can be "128" which is 4
bytes including the \0 char.
And you should also update the documentation regarding the fact that
prefix length will overwrite your netmask setting.
Thanks,
Daniel