Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Online check fails for working Internet connection
(Robert Tiemann)
2. Re: Tethering - using different subnets (Daniel Wagner)
3. [PATCH] rootnfs: Working rootnfs using connman (Pantelis Antoniou)
4. Re: [PATCH] rootnfs: Working rootnfs using connman (Sam Nazarko)
5. Re: [PATCH] rootnfs: Working rootnfs using connman (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Wed, 30 Nov 2016 10:22:04 +0100
From: Robert Tiemann <[email protected]>
To: [email protected]
Subject: Online check fails for working Internet connection
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi,
I've run into a situation in which ConnMan's online check often fails
due to a temporary HTTP 404 error. ConnMan remains in ready state and
never repeats the online check. The connection, however, is actually
correctly set up and works.
As I've found out, the 404 error is actually a DNS timeout that is
reported as 404 by resolv_result(). It happens right after
establishing a WLAN connection to a slow access point using manual
IPv4 configuration (no DHCP involved here). Successive DNS queries are
answered more or less immediately, it's just the first one that is
always slow, often taking longer than the timeout of 5 seconds. Even
worse: sometimes the first DNS query is simply dropped by that
particular access point, so there will never be an answer, no matter
how long the timeout would be set.
While I think this problem is clearly caused by the access point and
ConnMan is not to blame here, repeating the online check would help a
lot. After all, the connection works well after the initial hiccups. I
can also dream up dozens of different scenarios that would lead to
similar temporary failure, and repeating the online check after
failure would fix these scenarios as well.
Note that there seems to be no way to fix this situation from external
applications as there is no way to request to restart the online
check. It is only possible to disconnect from the service and connect
again, but it doesn't help here because the DNS timeout problem
potentially occurs again on each connect.
Is there a reason to avoid repeating the online check after failure?
Are there any plans for improving the online check?
Best regards,
Robert Tiemann
------------------------------
Message: 2
Date: Wed, 30 Nov 2016 10:10:52 +0100
From: Daniel Wagner <[email protected]>
To: Usman S <[email protected]>, [email protected]
Subject: Re: Tethering - using different subnets
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Usman,
On 11/29/2016 08:14 PM, Usman S wrote:
> Thanks for the response!! I was using v1.25 and the patch has been
> integrated in v1.31 and I will update to a newer version.
Along with getting more stable IP blocks you get a bunch of bug fixes as
well (and hopefully no regressions :)).
> As mentioned
> in the patch, the same block will be only offered if it was not
> allocated for any other device. Should we think of a way where we
> maintain the offered blocks for a particular device so that next time
> the same is offered or more freedom to the application in choosing its
> subnet?
Of course we could implement something like this but I would like to
know why is that so important? Is it because you have static IP
addresses assigned to devices which use the uplink?
cheers,
daniel
------------------------------
Message: 3
Date: Wed, 30 Nov 2016 15:43:39 +0200
From: Pantelis Antoniou <[email protected]>
To: [email protected]
Cc: Marcel Holtmann <[email protected]>, Jukka Rissanen
<[email protected]>, Stephane Desneux
<[email protected]>, Koen Kooi <[email protected]>, Pantelis
Antoniou <[email protected]>
Subject: [PATCH] rootnfs: Working rootnfs using connman
Message-ID:
<[email protected]>
Until now for root NFS you either had to manually blacklist
the interface or disable connman all together
This patch automatically blacklists the interface the NFS server
is reachable from and populates the resolver entries that the
DHCP server provided on startup.
It is now possible to use a vanilla rootfs tarball without
having to manually edit connman configuration entries.
Signed-off-by: Pantelis Antoniou <[email protected]>
---
src/connman.h | 3 +
src/device.c | 8 +-
src/inet.c | 281 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/resolver.c | 8 ++
4 files changed, 299 insertions(+), 1 deletion(-)
diff --git a/src/connman.h b/src/connman.h
index ce3ef8d..1a76dc2 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -244,6 +244,9 @@ int __connman_inet_del_default_from_table(uint32_t
table_id, int ifindex, const
int __connman_inet_get_address_netmask(int ifindex,
struct sockaddr_in *address, struct sockaddr_in *netmask);
+bool __connman_inet_isrootnfs_device(const char *devname);
+gchar **__connman_inet_get_pnp_nameservers(const char *pnp_file);
+
#include <connman/resolver.h>
int __connman_resolver_init(gboolean dnsproxy);
diff --git a/src/device.c b/src/device.c
index 742b3c4..3bcf5ab 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1324,7 +1324,7 @@ list:
blacklisted_interfaces =
connman_setting_get_string_list("NetworkInterfaceBlacklist");
if (!blacklisted_interfaces)
- return false;
+ goto rootnfs;
for (pattern = blacklisted_interfaces; *pattern; pattern++) {
if (g_str_has_prefix(devname, *pattern)) {
@@ -1333,6 +1333,12 @@ list:
}
}
+rootnfs:
+ if (__connman_inet_isrootnfs_device(devname)) {
+ DBG("ignoring device %s (rootnfs)", devname);
+ return true;
+ }
+
return false;
}
diff --git a/src/inet.c b/src/inet.c
index 803f0e6..6733bfa 100644
--- a/src/inet.c
+++ b/src/inet.c
@@ -2964,3 +2964,284 @@ out:
close(sk);
return ret;
}
+
+static int get_nfs_server_ip(const char *cmdline_file, const char *pnp_file,
+ struct in_addr *addr)
+{
+ GError *error = NULL;
+ gchar *cmdline = NULL;
+ gchar *pnp = NULL;
+ gchar **args = NULL;
+ gchar **pnpent = NULL;
+ gchar **pp = NULL;
+ gchar *s, *nfsargs;
+ int ret, len, err;
+ char addrstr[INET_ADDRSTRLEN];
+ struct in_addr taddr;
+
+ ret = -1;
+
+ if (!cmdline_file)
+ cmdline_file = "/proc/cmdline";
+ if (!pnp_file)
+ pnp_file = "/proc/net/pnp";
+ if (!addr)
+ addr = &taddr;
+ addr->s_addr = INADDR_NONE;
+
+ if (!g_file_get_contents(cmdline_file, &cmdline, NULL, &error)) {
+ g_printerr("Cannot read %s %s\n", cmdline_file, error->message);
+ goto out;
+ }
+
+ if (!g_file_get_contents(pnp_file, &pnp, NULL, &error)) {
+ g_printerr("Cannot read %s %s\n", pnp_file, error->message);
+ goto out;
+ }
+
+ len = strlen(cmdline);
+ if (len <= 1) {
+ /* too short */
+ goto out;
+ }
+ /* remove newline */
+ if (cmdline[len - 1] == '\n')
+ cmdline[--len] = '\0';
+
+ /* g_print("cmdline=%s\n", cmdline); */
+
+ /* split in arguments (seperated by space) */
+ args = g_strsplit(cmdline, " ", 0);
+ if (!args) {
+ g_printerr("Cannot split cmdline \"%s\"\n", cmdline);
+ goto out;
+ }
+
+ /* split in entries (by newlines) */
+ pnpent = g_strsplit(pnp, "\n", 0);
+ if (!pnpent) {
+ g_printerr("Cannot split pnp\n");
+ goto out;
+ }
+
+ /* first find root argument */
+ for (pp = args; *pp; pp++) {
+ if (!strcmp(*pp, "root=/dev/nfs"))
+ break;
+ }
+ /* no rootnfs found */
+ if (!*pp)
+ goto out;
+
+ /* g_print("root argument: %s\n", *pp); */
+
+ /* locate nfsroot argument */
+ for (pp = args; *pp; pp++) {
+ if (!strncmp(*pp, "nfsroot=", strlen("nfsroot=")))
+ break;
+ }
+ /* no nfsroot argument found */
+ if (!*pp)
+ goto out;
+
+ /* g_print("nfsroot argument: %s\n", *pp); */
+
+ /* determine if nfsroot server is provided */
+ nfsargs = strchr(*pp, '=');
+ if (!nfsargs)
+ goto out;
+ nfsargs++;
+
+ /* find whether serverip is present */
+ s = strchr(nfsargs, ':');
+ if (s) {
+ len = s - nfsargs;
+ s = nfsargs;
+ } else {
+ /* no serverip, use bootserver */
+ for (pp = pnpent; *pp; pp++) {
+ if (!strncmp(*pp, "bootserver ", strlen("bootserver ")))
+ break;
+ }
+ /* no bootserver found */
+ if (!*pp)
+ goto out;
+ s = *pp + strlen("bootserver ");
+ len = strlen(s);
+ }
+
+ /* copy to addr string buffer */
+ if (len >= sizeof(addrstr)) {
+ g_printerr("Bad server\n");
+ goto out;
+ }
+ memcpy(addrstr, s, len);
+ addrstr[len] = '\0';
+
+ /* g_print("using server addr: %s\n", addrstr); */
+
+ err = inet_pton(AF_INET, addrstr, addr);
+ if (err <= 0) {
+ g_printerr("Cannot convert to numeric addr \"%s\"\n",
+ addrstr);
+ goto out;
+ }
+ /* all done */
+ ret = 0;
+out:
+ g_strfreev(pnpent);
+ g_strfreev(args);
+ if (error)
+ g_error_free(error);
+ g_free(pnp);
+ g_free(cmdline);
+
+ return ret;
+}
+
+/* get interface out of which peer is reachable (IPv4 only) */
+static int get_peer_iface(struct in_addr *addr, char *ifname)
+{
+ struct ifaddrs *ifaddr, *ifa;
+ int ret = -1, s;
+ socklen_t socklen;
+ struct sockaddr_in saddr, *ifsaddr;
+
+ /* Obtain address(es) matching host/port */
+ ret = getifaddrs(&ifaddr);
+ if (ret == -1) {
+ fprintf(stderr, "getifaddrs() failed %d (%s)\n",
+ errno, strerror(errno));
+ return -1;
+ }
+
+ s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+ if (s == -1) {
+ fprintf(stderr, "socket() failed %d (%s)\n",
+ errno, strerror(errno));
+ return -1;
+ }
+
+ memset(&saddr, 0, sizeof(saddr));
+ saddr.sin_family = AF_INET;
+ saddr.sin_port = 0; /* any port */
+ saddr.sin_addr = *addr;
+
+ /* no need to bind, connect will select iface */
+ ret = connect(s, (struct sockaddr *)&saddr, sizeof(struct sockaddr_in));
+ if (ret == -1) {
+ fprintf(stderr, "connect() failed: %d (%s)\n",
+ errno, strerror(errno));
+ goto do_close;
+ }
+
+ socklen = sizeof(saddr);
+ ret = getsockname(s, (struct sockaddr *)&saddr, &socklen);
+ if (ret == -1) {
+ fprintf(stderr, "getsockname() failed: %d (%s)\n",
+ errno, strerror(errno));
+ goto do_close;
+ }
+
+ ret = -1;
+ for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) {
+ if (ifa->ifa_addr == NULL)
+ continue;
+
+ /* only IPv4 address */
+ if (ifa->ifa_addr->sa_family != AF_INET)
+ continue;
+
+ ifsaddr = (struct sockaddr_in *)ifa->ifa_addr;
+
+ /* match address? */
+ if (ifsaddr->sin_addr.s_addr == saddr.sin_addr.s_addr)
+ break;
+ }
+
+ if (ifa) {
+ ret = 0;
+ if (ifname)
+ strcpy(ifname, ifa->ifa_name);
+ }
+
+do_close:
+ close(s);
+
+ freeifaddrs(ifaddr);
+
+ return ret;
+}
+
+bool __connman_inet_isrootnfs_device(const char *devname)
+{
+ struct in_addr addr;
+ char ifname[IFNAMSIZ];
+
+ return get_nfs_server_ip(NULL, NULL, &addr) == 0 &&
+ get_peer_iface(&addr, ifname) == 0 &&
+ strcmp(devname, ifname) == 0;
+}
+
+gchar **__connman_inet_get_pnp_nameservers(const char *pnp_file)
+{
+ GError *error = NULL;
+ gchar *pnp = NULL;
+ gchar **pnpent = NULL;
+ gchar **nameservers = NULL;
+ gchar **pp;
+ char *s;
+ int pass, count;
+
+ if (!pnp_file)
+ pnp_file = "/proc/net/pnp";
+
+ if (!g_file_get_contents(pnp_file, &pnp, NULL, &error)) {
+ g_printerr("Cannot read %s %s\n", pnp_file, error->message);
+ goto out;
+ }
+
+ /* split in entries (by newlines) */
+ pnpent = g_strsplit(pnp, "\n", 0);
+ if (!pnpent) {
+ g_printerr("Cannot split pnp\n");
+ goto out;
+ }
+
+ count = 0;
+ nameservers = NULL;
+ for (pass = 1; pass <= 2; pass++) {
+
+ if (pass == 2)
+ nameservers = g_new(gchar *, count + 1);
+
+ count = 0;
+ for (pp = pnpent; *pp; pp++) {
+ if (strncmp(*pp, "nameserver ", strlen("nameserver ")))
+ continue;
+ s = *pp + strlen("nameserver ");
+ if (!strcmp(s, "0.0.0.0"))
+ continue;
+
+ if (pass == 2)
+ nameservers[count] = g_strdup(s);
+ count++;
+ }
+
+ /* no nameservers? */
+ if (count == 0)
+ goto out;
+
+ if (pass == 2)
+ nameservers[count] = NULL;
+
+ }
+
+out:
+ g_strfreev(pnpent);
+ g_free(pnp);
+ if (error)
+ g_error_free(error);
+
+ return nameservers;
+}
diff --git a/src/resolver.c b/src/resolver.c
index c4adbc6..75ea5ba 100644
--- a/src/resolver.c
+++ b/src/resolver.c
@@ -659,6 +659,14 @@ int __connman_resolver_init(gboolean dnsproxy)
DBG("dnsproxy %d", dnsproxy);
+ /* get autoip nameservers */
+ ns = __connman_inet_get_pnp_nameservers(NULL);
+ for (i = 0; ns && ns[i]; i += 1) {
+ DBG("pnp server %s", ns[i]);
+ append_resolver(i, NULL, ns[i], 86400, 0);
+ }
+ g_strfreev(ns);
+
if (!dnsproxy)
return 0;
--
2.1.4
------------------------------
Message: 4
Date: Wed, 30 Nov 2016 14:25:53 +0000
From: Sam Nazarko <[email protected]>
To: Pantelis Antoniou <[email protected]>,
"[email protected]" <[email protected]>
Cc: Koen Kooi <[email protected]>, Stephane Desneux
<[email protected]>
Subject: Re: [PATCH] rootnfs: Working rootnfs using connman
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"
________________________________________
From: connman <[email protected]> on behalf of Pantelis Antoniou
<[email protected]>
Sent: 30 November 2016 13:43
To: [email protected]
Cc: Koen Kooi; Stephane Desneux
Subject: [PATCH] rootnfs: Working rootnfs using connman
Until now for root NFS you either had to manually blacklist
the interface or disable connman all together
This patch automatically blacklists the interface the NFS server
is reachable from and populates the resolver entries that the
DHCP server provided on startup.
It is now possible to use a vanilla rootfs tarball without
having to manually edit connman configuration entries.
_______________________________________________
Nice.
I was blacklisting and populating nameservers from /proc/net/pnp before; but
this is a much cleaner approach. I'll give it some testing with OSMC.
Sam
------------------------------
Message: 5
Date: Wed, 30 Nov 2016 18:30:21 +0100
From: Daniel Wagner <[email protected]>
To: Pantelis Antoniou <[email protected]>,
[email protected]
Cc: Koen Kooi <[email protected]>, Stephane Desneux
<[email protected]>
Subject: Re: [PATCH] rootnfs: Working rootnfs using connman
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Pantelis,
On 11/30/2016 02:43 PM, Pantelis Antoniou wrote:
> Until now for root NFS you either had to manually blacklist
> the interface or disable connman all together
>
> This patch automatically blacklists the interface the NFS server
> is reachable from and populates the resolver entries that the
> DHCP server provided on startup.
>
> It is now possible to use a vanilla rootfs tarball without
> having to manually edit connman configuration entries.
>
> Signed-off-by: Pantelis Antoniou <[email protected]>
We don't do the SoB.
> ---
> src/connman.h | 3 +
> src/device.c | 8 +-
> src/inet.c | 281
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/resolver.c | 8 ++
> 4 files changed, 299 insertions(+), 1 deletion(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index ce3ef8d..1a76dc2 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -244,6 +244,9 @@ int __connman_inet_del_default_from_table(uint32_t
> table_id, int ifindex, const
> int __connman_inet_get_address_netmask(int ifindex,
> struct sockaddr_in *address, struct sockaddr_in *netmask);
>
> +bool __connman_inet_isrootnfs_device(const char *devname);
> +gchar **__connman_inet_get_pnp_nameservers(const char *pnp_file);
Just use plain 'char' here. We try to avoid get infested by all the glib
types.
> +
> #include <connman/resolver.h>
>
> int __connman_resolver_init(gboolean dnsproxy);
> diff --git a/src/device.c b/src/device.c
> index 742b3c4..3bcf5ab 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1324,7 +1324,7 @@ list:
move the rootnfs test (1) here.
> blacklisted_interfaces =
> connman_setting_get_string_list("NetworkInterfaceBlacklist");
> if (!blacklisted_interfaces)
> - return false;
> + goto rootnfs;
and this is not needed.
>
> for (pattern = blacklisted_interfaces; *pattern; pattern++) {
> if (g_str_has_prefix(devname, *pattern)) {
> @@ -1333,6 +1333,12 @@ list:
> }
> }
>
> +rootnfs:
> + if (__connman_inet_isrootnfs_device(devname)) {
> + DBG("ignoring device %s (rootnfs)", devname);
> + return true;
> + }
> +
(1) this part here
> return false;
> }
>
> diff --git a/src/inet.c b/src/inet.c
> index 803f0e6..6733bfa 100644
> --- a/src/inet.c
> +++ b/src/inet.c
> @@ -2964,3 +2964,284 @@ out:
> close(sk);
> return ret;
> }
> +
> +static int get_nfs_server_ip(const char *cmdline_file, const char *pnp_file,
> + struct in_addr *addr)
> +{
> + GError *error = NULL;
> + gchar *cmdline = NULL;
> + gchar *pnp = NULL;
> + gchar **args = NULL;
> + gchar **pnpent = NULL;
> + gchar **pp = NULL;
> + gchar *s, *nfsargs;
> + int ret, len, err;
> + char addrstr[INET_ADDRSTRLEN];
> + struct in_addr taddr;
Again use char if possible here. Also order the list, first the non
initializing part (decleration only) followed by the ones with the
initilization.
> +
> + ret = -1;
> +
We use 'err' instead of 'ret' almost everywhere. Also use something
meaning full
as error code, EINVAL or so.
> + if (!cmdline_file)
> + cmdline_file = "/proc/cmdline";
> + if (!pnp_file)
> + pnp_file = "/proc/net/pnp";
> + if (!addr)
> + addr = &taddr;
> + addr->s_addr = INADDR_NONE;
> +
> + if (!g_file_get_contents(cmdline_file, &cmdline, NULL, &error)) {
> + g_printerr("Cannot read %s %s\n", cmdline_file, error->message);
Use connman_error() for this.
> + goto out;
> + }
Indention looks wrong? There are some more occurrences below. I don't
want to repeat for each line. So just make sure the whitespace stuff is
okay.
> +
> + if (!g_file_get_contents(pnp_file, &pnp, NULL, &error)) {
> + g_printerr("Cannot read %s %s\n", pnp_file, error->message);
connman_error(), and the following g_printerr() too.
> + goto out;
> + }
> +
> + len = strlen(cmdline);
> + if (len <= 1) {
> + /* too short */
> + goto out;
> + }
> + /* remove newline */
> + if (cmdline[len - 1] == '\n')
> + cmdline[--len] = '\0';
> +
> + /* g_print("cmdline=%s\n", cmdline); */
Just remove it the debug stuff. In the past we had too many of those low
level debug prints everywhere. During development its fine but when
ready for upstream just get rid of it.
> +
> + /* split in arguments (seperated by space) */
> + args = g_strsplit(cmdline, " ", 0);
> + if (!args) {
> + g_printerr("Cannot split cmdline \"%s\"\n", cmdline);
> + goto out;
> + }
> +
> + /* split in entries (by newlines) */
> + pnpent = g_strsplit(pnp, "\n", 0);
> + if (!pnpent) {
> + g_printerr("Cannot split pnp\n");
> + goto out;
> + }
> +
> + /* first find root argument */
> + for (pp = args; *pp; pp++) {
> + if (!strcmp(*pp, "root=/dev/nfs"))
> + break;
> + }
> + /* no rootnfs found */
> + if (!*pp)
> + goto out;
> +
> + /* g_print("root argument: %s\n", *pp); */
remove
> +
> + /* locate nfsroot argument */
> + for (pp = args; *pp; pp++) {
> + if (!strncmp(*pp, "nfsroot=", strlen("nfsroot=")))
> + break;
> + }
> + /* no nfsroot argument found */
> + if (!*pp)
> + goto out;
> +
> + /* g_print("nfsroot argument: %s\n", *pp); */
you know what I want to write :)
> +
> + /* determine if nfsroot server is provided */
> + nfsargs = strchr(*pp, '=');
> + if (!nfsargs)
> + goto out;
> + nfsargs++;
> +
> + /* find whether serverip is present */
> + s = strchr(nfsargs, ':');
> + if (s) {
> + len = s - nfsargs;
> + s = nfsargs;
> + } else {
> + /* no serverip, use bootserver */
> + for (pp = pnpent; *pp; pp++) {
> + if (!strncmp(*pp, "bootserver ", strlen("bootserver ")))
> + break;
> + }
> + /* no bootserver found */
> + if (!*pp)
> + goto out;
> + s = *pp + strlen("bootserver ");
> + len = strlen(s);
> + }
> +
> + /* copy to addr string buffer */
> + if (len >= sizeof(addrstr)) {
> + g_printerr("Bad server\n");
> + goto out;
> + }
> + memcpy(addrstr, s, len);
> + addrstr[len] = '\0';
> +
> + /* g_print("using server addr: %s\n", addrstr); */
> +
> + err = inet_pton(AF_INET, addrstr, addr);
> + if (err <= 0) {
> + g_printerr("Cannot convert to numeric addr \"%s\"\n",
> + addrstr);
> + goto out;
> + }
> + /* all done */
> + ret = 0;
> +out:
> + g_strfreev(pnpent);
> + g_strfreev(args);
> + if (error)
> + g_error_free(error);
> + g_free(pnp);
> + g_free(cmdline);
> +
> + return ret;
> +}
> +
> +/* get interface out of which peer is reachable (IPv4 only) */
> +static int get_peer_iface(struct in_addr *addr, char *ifname)
> +{
> + struct ifaddrs *ifaddr, *ifa;
> + int ret = -1, s;
> + socklen_t socklen;
> + struct sockaddr_in saddr, *ifsaddr;
same as in in get_nfs_server_ip()
> +
> + /* Obtain address(es) matching host/port */
> + ret = getifaddrs(&ifaddr);
> + if (ret == -1) {
we use 'if (err < 0)'
> + fprintf(stderr, "getifaddrs() failed %d (%s)\n",
> + errno, strerror(errno));
connman_error()
> + return -1;
> + }
> +
> + s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> + if (s == -1) {
> + fprintf(stderr, "socket() failed %d (%s)\n",
> + errno, strerror(errno));
> + return -1;
> + }
> +
> + memset(&saddr, 0, sizeof(saddr));
> + saddr.sin_family = AF_INET;
> + saddr.sin_port = 0; /* any port */
> + saddr.sin_addr = *addr;
> +
> + /* no need to bind, connect will select iface */
> + ret = connect(s, (struct sockaddr *)&saddr, sizeof(struct sockaddr_in));
> + if (ret == -1) {
if (err < 0)
> + fprintf(stderr, "connect() failed: %d (%s)\n",
> + errno, strerror(errno));
connman_error()
> + goto do_close;
> + }
> +
> + socklen = sizeof(saddr);
> + ret = getsockname(s, (struct sockaddr *)&saddr, &socklen);
> + if (ret == -1) {
> + fprintf(stderr, "getsockname() failed: %d (%s)\n",
> + errno, strerror(errno));
> + goto do_close;
Since in this file we have either 'out' or 'done' as label I suggest you
use it too. Nothing wrong with do_close it just stands out in this file.
> + }
> +
> + ret = -1;
> + for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) {
for (...; ifa; ...)
no need to do the != NULL test
> + if (ifa->ifa_addr == NULL)
> + continue;
if (!ifa->ifa_addr)
> +
> + /* only IPv4 address */
> + if (ifa->ifa_addr->sa_family != AF_INET)
> + continue;
> +
> + ifsaddr = (struct sockaddr_in *)ifa->ifa_addr;
> +
> + /* match address? */
> + if (ifsaddr->sin_addr.s_addr == saddr.sin_addr.s_addr)
> + break;
> + }
> +
> + if (ifa) {
> + ret = 0;
> + if (ifname)
> + strcpy(ifname, ifa->ifa_name);
> + }
> +
> +do_close:
> + close(s);
> +
> + freeifaddrs(ifaddr);
> +
> + return ret;
> +}
> +
> +bool __connman_inet_isrootnfs_device(const char *devname)
> +{
> + struct in_addr addr;
> + char ifname[IFNAMSIZ];
> +
> + return get_nfs_server_ip(NULL, NULL, &addr) == 0 &&
> + get_peer_iface(&addr, ifname) == 0 &&
> + strcmp(devname, ifname) == 0;
> +}
> +
> +gchar **__connman_inet_get_pnp_nameservers(const char *pnp_file)
> +{
> + GError *error = NULL;
> + gchar *pnp = NULL;
> + gchar **pnpent = NULL;
> + gchar **nameservers = NULL;
> + gchar **pp;
> + char *s;
> + int pass, count;
char
> +
> + if (!pnp_file)
> + pnp_file = "/proc/net/pnp";
> +
> + if (!g_file_get_contents(pnp_file, &pnp, NULL, &error)) {
> + g_printerr("Cannot read %s %s\n", pnp_file, error->message);
> + goto out;
> + }
> +
> + /* split in entries (by newlines) */
> + pnpent = g_strsplit(pnp, "\n", 0);
> + if (!pnpent) {
> + g_printerr("Cannot split pnp\n");
> + goto out;
> + }
> +
> + count = 0;
> + nameservers = NULL;
> + for (pass = 1; pass <= 2; pass++) {
> +
> + if (pass == 2)
> + nameservers = g_new(gchar *, count + 1);
> +
> + count = 0;
> + for (pp = pnpent; *pp; pp++) {
> + if (strncmp(*pp, "nameserver ", strlen("nameserver ")))
> + continue;
> + s = *pp + strlen("nameserver ");
> + if (!strcmp(s, "0.0.0.0"))
> + continue;
> +
> + if (pass == 2)
> + nameservers[count] = g_strdup(s);
> + count++;
> + }
> +
> + /* no nameservers? */
> + if (count == 0)
> + goto out;
> +
> + if (pass == 2)
> + nameservers[count] = NULL;
> +
> + }
So I was starring at this loop for a couple of minutes and I don't get
it. You have added comments to many obvious code snippets like the one
above (split in entries). Even though one can argue about the value of
those comments, here it would be really helpful. What is happening here?
> +
> +out:
> + g_strfreev(pnpent);
> + g_free(pnp);
> + if (error)
> + g_error_free(error);
> +
> + return nameservers;
> +}
> diff --git a/src/resolver.c b/src/resolver.c
> index c4adbc6..75ea5ba 100644
> --- a/src/resolver.c
> +++ b/src/resolver.c
> @@ -659,6 +659,14 @@ int __connman_resolver_init(gboolean dnsproxy)
>
> DBG("dnsproxy %d", dnsproxy);
>
> + /* get autoip nameservers */
> + ns = __connman_inet_get_pnp_nameservers(NULL);
> + for (i = 0; ns && ns[i]; i += 1) {
> + DBG("pnp server %s", ns[i]);
> + append_resolver(i, NULL, ns[i], 86400, 0);
> + }
> + g_strfreev(ns);
> +
> if (!dnsproxy)
> return 0;
>
>
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 13, Issue 34
***************************************
