Send connman mailing list submissions to
connman@lists.01.org
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
connman-requ...@lists.01.org
You can reach the person managing the list at
connman-ow...@lists.01.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH] technology: avoid redundant saving to the file system
(Yusuke Nakamura)
2. Re: [PATCH 04/16] gdhcp: Fix potential NULL deref
(Tomasz Bursztyka)
3. Re: [PATCH 10/16] util: Reading from /dev/urandom ignores the
number of bytes read (Tomasz Bursztyka)
4. Re: [PATCH 06/16] peer_service: Setting retval ignored,
always overwritten (Tomasz Bursztyka)
5. PreferredTechonologies for 'ready' services (Maxime CHEVALLIER)
6. Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval (Patrik Flykt)
7. Re: [PATCH 02/16] common: Check setsockopt() retval (Patrik Flykt)
8. Re: [PATCH 05/16] ofono: Fix potential NULL deref (Patrik Flykt)
9. Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval (Patrik Flykt)
10. Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval
(Peter Meerwald-Stadler)
----------------------------------------------------------------------
Message: 1
Date: Mon, 19 Sep 2016 14:32:35 +0900
From: Yusuke Nakamura <yusuke1653...@gmail.com>
To: connman@lists.01.org
Subject: [PATCH] technology: avoid redundant saving to the file system
Message-ID:
<1474263155-4855-1-git-send-email-ynakam...@jp.adit-jv.com>
Even if OfflineMode is the same as before, Connman always saves
to the file system. This patch makes Connman to save OfflineMode
only when it is actually changed in order to avoid redundant data
writing.
---
src/technology.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/technology.c b/src/technology.c
index 660af52..0fc2cb2 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -444,15 +444,31 @@ bool __connman_technology_get_offlinemode(void)
static void connman_technology_save_offlinemode(void)
{
GKeyFile *keyfile;
+ GError *error = NULL;
+ bool offlinemode;
keyfile = __connman_storage_load_global();
- if (!keyfile)
+
+ if (!keyfile) {
keyfile = g_key_file_new();
+ g_key_file_set_boolean(keyfile, "global",
+ "OfflineMode", global_offlinemode);
- g_key_file_set_boolean(keyfile, "global",
+ __connman_storage_save_global(keyfile);
+ }
+ else {
+ offlinemode = g_key_file_get_boolean(keyfile, "global",
+ "OfflineMode", &error);
+
+ if (error || offlinemode != global_offlinemode) {
+ g_key_file_set_boolean(keyfile, "global",
"OfflineMode", global_offlinemode);
+ if (error)
+ g_clear_error(&error);
- __connman_storage_save_global(keyfile);
+ __connman_storage_save_global(keyfile);
+ }
+ }
g_key_file_free(keyfile);
--
1.7.9.5
------------------------------
Message: 2
Date: Mon, 19 Sep 2016 08:39:04 +0200
From: Tomasz Bursztyka <tomasz.burszt...@linux.intel.com>
To: connman@lists.01.org
Subject: Re: [PATCH 04/16] gdhcp: Fix potential NULL deref
Message-ID: <d04d5cd2-f174-42ea-d79e-d5fe40786...@linux.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Peter,
> packet6 is not set if dhcpv6_recv_l3_packet() returns an error
>
> CID 1352473
> ---
> gdhcp/client.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gdhcp/client.c b/gdhcp/client.c
> index e9e38e7..af1b953 100644
> --- a/gdhcp/client.c
> +++ b/gdhcp/client.c
> @@ -2289,6 +2289,8 @@ static gboolean listener_event(GIOChannel *channel,
> GIOCondition condition,
> if (dhcp_client->type == G_DHCP_IPV6) {
> re = dhcpv6_recv_l3_packet(&packet6, buf, sizeof(buf),
> dhcp_client->listener_sockfd);
> + if (re < 0)
> + return TRUE;
Add an empty line right after.
> pkt_len = re;
> pkt = packet6;
> xid = packet6->transaction_id[0] << 16 |
------------------------------
Message: 3
Date: Mon, 19 Sep 2016 08:43:38 +0200
From: Tomasz Bursztyka <tomasz.burszt...@linux.intel.com>
To: connman@lists.01.org
Subject: Re: [PATCH 10/16] util: Reading from /dev/urandom ignores the
number of bytes read
Message-ID: <559493e5-982d-550e-8143-1fd857aed...@linux.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Peter,
This patch might be unnecessary as your patch 16 is replacing it altogether.
Tomasz
------------------------------
Message: 4
Date: Mon, 19 Sep 2016 08:47:29 +0200
From: Tomasz Bursztyka <tomasz.burszt...@linux.intel.com>
To: connman@lists.01.org
Subject: Re: [PATCH 06/16] peer_service: Setting retval ignored,
always overwritten
Message-ID: <ef5d1e19-7008-b2bd-115d-d823a656a...@linux.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Peter,
> CID 1352484
> ---
> src/peer_service.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/src/peer_service.c b/src/peer_service.c
> index 053672a..a457bff 100644
> --- a/src/peer_service.c
> +++ b/src/peer_service.c
> @@ -293,9 +293,6 @@ int __connman_peer_service_register(const char *owner,
> DBusMessage *msg,
> if (service) {
> DBG("Found one existing service %p", service);
>
> - if (g_strcmp0(service->owner, owner))
> - ret = -EBUSY;
> -
Actually, the right fix should be:
if (...) {
ret = -EBUSY;
goto error;
}
> if (service->pending)
> ret = -EINPROGRESS;
> else
------------------------------
Message: 5
Date: Mon, 19 Sep 2016 10:53:13 +0200
From: Maxime CHEVALLIER <maxime.chevall...@smile.fr>
To: <connman@lists.01.org>
Subject: PreferredTechonologies for 'ready' services
Message-ID: <2b5f12a90e5a36b6bca8456802346...@smile.fr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Hi all,
Following up on my previous post [1], I found this in the documentation
:
The doc 'connman.conf.5.in' specifies, for PreferredTechnologies :
"A service of a preferred technology type in state 'ready' will get the
default route when compared to another preferred type further down
the
list with state 'ready' or with a non-preferred type"
However this does not seem to be true :
The 'service_compare' function in src/service.c doesn't take the
PreferredTechnologies list into account when sorting services.
In my case ( cellular and ethernet 'ready', but never 'online' ), this
prevents me from having the 'cellular' technology as the default
service,
ethernet always goes back to the top of the list after a while :
my /etc/connman/main.conf :
[General]
PreferredTechnologies=cellular,ethernet
Output of connmanctl services 30 seconds after booting :
*AR Wired ethernet_0010025f0148_p00_cable
*AR DATA ONLY (F SFR) cellular_000000000000072_context1
*Ac Wired ethernet_0010025f0148_cable
I was wondering if this was a wrong description in the documentation,
or
if this was an actual missing check in the implementation.
I can work on a patch if needed, but that would be great if you can
confirm that this is an actual issue, and not me misinterpreting the
doc or the code.
Best regards,
Maxime Chevallier
[1] : https://lists.01.org/pipermail/connman/2016-September/020900.html
------------------------------
Message: 6
Date: Mon, 19 Sep 2016 16:10:02 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Peter Meerwald-Stadler <pme...@pmeerw.net>
Cc: connman@lists.01.org
Subject: Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval
Message-ID: <1474290602.3482.7.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
On Sat, 2016-09-17 at 13:05 +0200, Peter Meerwald-Stadler wrote:
> CID 1352468
Looks like Coverity, but some reference to the scan could be made here?
> ---
> ?gdhcp/common.c | 5 ++++-
> ?1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gdhcp/common.c b/gdhcp/common.c
> index 3817bcc..b23cadf 100644
> --- a/gdhcp/common.c
> +++ b/gdhcp/common.c
> @@ -490,7 +490,10 @@ int dhcpv6_send_packet(int index, struct
> dhcpv6_packet *dhcp_pkt, int len)
> ? if (fd < 0)
> ? return -errno;
> ?
> - setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
> + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt,
> sizeof(opt)) < 0) {
> + close(fd);
> + return -errno;
> + }
With this, the return value is the errno for close(), which is zero on
success and < 0 if close fails. Needs fixing by remembering the errno
from setsockopt() in another variable, call close() and return the
other variable instead?
Cheers,
Patrik
> ?
> ? memset(&src, 0, sizeof(src));
> ? src.sin6_family = AF_INET6;
------------------------------
Message: 7
Date: Mon, 19 Sep 2016 16:10:42 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Peter Meerwald-Stadler <pme...@pmeerw.net>
Cc: connman@lists.01.org
Subject: Re: [PATCH 02/16] common: Check setsockopt() retval
Message-ID: <1474290642.3482.8.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
On Sat, 2016-09-17 at 13:05 +0200, Peter Meerwald-Stadler wrote:
> CID 1075205, 1075204
> ---
> ?gdhcp/common.c | 10 ++++++++--
> ?1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/gdhcp/common.c b/gdhcp/common.c
> index b23cadf..3cc6a09 100644
> --- a/gdhcp/common.c
> +++ b/gdhcp/common.c
> @@ -644,7 +644,10 @@ int dhcp_send_kernel_packet(struct dhcp_packet
> *dhcp_pkt,
> ? if (fd < 0)
> ? return -errno;
> ?
> - setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
> + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt,
> sizeof(opt)) < 0) {
> + close(fd);
> + return -errno;
> + }
> ?
> ? memset(&client, 0, sizeof(client));
> ? client.sin_family = AF_INET;
> @@ -685,7 +688,10 @@ int dhcp_l3_socket(int port, const char
> *interface, int family)
> ? if (fd < 0)
> ? return -errno;
> ?
> - setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
> + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt,
> sizeof(opt)) < 0) {
> + ????close(fd);
> + ????return -errno;
> + }
As in patch 1/16.
Patrik
> ?
> ? if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE,
> ? interface, strlen(interface) + 1) <
> 0) {
------------------------------
Message: 8
Date: Mon, 19 Sep 2016 16:28:00 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Peter Meerwald-Stadler <pme...@pmeerw.net>
Cc: connman@lists.01.org
Subject: Re: [PATCH 05/16] ofono: Fix potential NULL deref
Message-ID: <1474291680.3482.10.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
On Sat, 2016-09-17 at 13:05 +0200, Peter Meerwald-Stadler wrote:
> could probably also remove
> if (!modem->context_list)
> and assert that modem->context_list is NULL
If it is known that add_cdma_network() is not called twice, the above
would be a better solution. Right now I don't know if that is the case,
either in practise or in this code...
Patrik
>
> CID 1352472
> ---
> ?plugins/ofono.c | 3 ++-
> ?1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/ofono.c b/plugins/ofono.c
> index 651cdaa..b6616e7 100644
> --- a/plugins/ofono.c
> +++ b/plugins/ofono.c
> @@ -1801,7 +1801,8 @@ static void add_cdma_network(struct modem_data
> *modem)
> ? context = network_context_alloc(modem->path);
> ? modem->context_list = g_slist_prepend(modem-
> >context_list,
> ? context);
> - }
> + } else
> + context = modem->context_list->data;
> ?
> ? if (!modem->name)
> ? modem->name = g_strdup("CDMA Network");
------------------------------
Message: 9
Date: Mon, 19 Sep 2016 16:30:15 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Peter Meerwald-Stadler <pme...@pmeerw.net>
Cc: connman@lists.01.org
Subject: Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval
Message-ID: <1474291815.3482.12.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
On Mon, 2016-09-19 at 16:10 +0300, Patrik Flykt wrote:
> On Sat, 2016-09-17 at 13:05 +0200, Peter Meerwald-Stadler wrote:
> > CID 1352468
>
> Looks like Coverity, but some reference to the scan could be made
> here?
Ok, I picked up the CIDs and as there are no really useful links, let's
keep the plain 'CID XXXX' as proposed.
Patrik
------------------------------
Message: 10
Date: Mon, 19 Sep 2016 15:43:12 +0200 (CEST)
From: Peter Meerwald-Stadler <pme...@pmeerw.net>
To: Patrik Flykt <patrik.fl...@linux.intel.com>
Cc: connman@lists.01.org
Subject: Re: [PATCH 01/16] dhcpv6: Check setsockopt() retval
Message-ID: <alpine.deb.2.02.1609191533220.1...@pmeerw.net>
Content-Type: text/plain; charset="iso-8859-15"
> On Sat, 2016-09-17 at 13:05 +0200, Peter Meerwald-Stadler wrote:
> > CID 1352468
>
> Looks like Coverity, but some reference to the scan could be made here?
a permalink to the issue would be nice; I have seen other projects just
state the CID
> > ---
> > ?gdhcp/common.c | 5 ++++-
> > ?1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdhcp/common.c b/gdhcp/common.c
> > index 3817bcc..b23cadf 100644
> > --- a/gdhcp/common.c
> > +++ b/gdhcp/common.c
> > @@ -490,7 +490,10 @@ int dhcpv6_send_packet(int index, struct
> > dhcpv6_packet *dhcp_pkt, int len)
> > ? if (fd < 0)
> > ? return -errno;
> > ?
> > - setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
> > + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt,
> > sizeof(opt)) < 0) {
> > + close(fd);
> > + return -errno;
> > + }
>
> With this, the return value is the errno for close(), which is zero on
> success and < 0 if close fails. Needs fixing by remembering the errno
> from setsockopt() in another variable, call close() and return the
> other variable instead?
right, thanks for having a close look :)
I'll spin a v2 of this series a little later
regards, p.
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
connman@lists.01.org
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 11, Issue 21
***************************************
