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. Re: [PATCH v2] gsupplicant: Updates to handling of security
change (Daniel Wagner)
2. Re: [PATCH] rtnl: Fix a debug print out (Daniel Wagner)
3. Re: When BackgroundScanning = false, connman's Scan() dbus
method is broken (Jose Blanquicet)
4. Re: [PATCH v2] gsupplicant: Updates to handling of security
change (Jose Blanquicet)
5. Re: [PATCH 1/2] resolver: add support for systemd-resolved.
(Jose Blanquicet)
----------------------------------------------------------------------
Message: 1
Date: Sun, 01 Oct 2017 15:48:03 +0200
From: Daniel Wagner <[email protected]>
To: Jose Blanquicet <[email protected]>
Cc: Harish Jenny K N <[email protected]>,
<[email protected]>
Subject: Re: [PATCH v2] gsupplicant: Updates to handling of security
change
Message-ID: <[email protected]>
Content-Type: text/plain
Harish Jenny K N <[email protected]> writes:
> Changing of security type of one BSS is resulting in removing of
> GSupplicantNetwork and other BSSs objects in the group.
> The corresponding entries for other BSS in the same group are not
> removed in other hash tables like bss_mapping and interface->bss_mapping.
> This commit only deletes the network when there is a single entry in
> network->bss_table during change in security for one bss.
> This commit also avoids accessing any deleted bss object when reply
> is got from async dbus call, by deleting any pending dbus calls
> associated with the bss object.
Jose, are you happy with this version? I'll apply it when I get your
ACK.
Thanks,
Daniel
------------------------------
Message: 2
Date: Sun, 1 Oct 2017 17:00:26 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>, [email protected]
Subject: Re: [PATCH] rtnl: Fix a debug print out
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jian,
On 09/06/2017 05:54 PM, Jian Liang wrote:
> From: Jian Liang <[email protected]>
>
> In <linux/netlink.h>, nlmsghdr is defined as
>
> struct nlmsghdr {
> __u32 nlmsg_len; /* Length of message including header. */
> __u16 nlmsg_type; /* Type of message content. */
> __u16 nlmsg_flags; /* Additional flags. */
> __u32 nlmsg_seq; /* Sequence number. */
> __u32 nlmsg_pid; /* Sender port ID. */
> };
>
> DBG print out should use %u instead of %d
Sorry for the delay with this one. I have missed to apply it. Patch
applied now.
Thanks,
Daniel
------------------------------
Message: 3
Date: Sun, 1 Oct 2017 20:13:37 +0200
From: Jose Blanquicet <[email protected]>
To: Jonah Petri <[email protected]>
Cc: Daniel Wagner <[email protected]>, [email protected]
Subject: Re: When BackgroundScanning = false, connman's Scan() dbus
method is broken
Message-ID:
<CAFC8iJ+eFh-d449jbS=ya3fjtkqphwoqdnal62nhqrmkezd...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Hi Jonah,
On Mon, Sep 25, 2017 at 8:58 PM, Jose Blanquicet <[email protected]> wrote:
> Follow an active scan with only one passive scan in case
> BackgroundScanning='false' is indeed the best solution. I thought I
> would be able to find time to work on that solution but it haven't
> yet, it has not been an ease time. This is the idea I have in mind:
>
> Current behaviour with BackgroundScanning='True' is:
> D-Bus scan request or at start-up of system
> Launch an active scan
> 3 seconds -> Launch passive scan
> 9 seconds -> Launch passive scan
> 27 seconds -> Launch passive scan
> 81 seconds -> Launch passive scan
> 243 seconds -> Launch passive scan
> 300 seconds -> Launch passive scan
> 300 seconds -> Launch passive scan
> ...
> Continue each 300 seconds unless you perform a new D-Bus scan call
> which will cause the procedure to restart from 3 seconds.
>
> With BackgroundScanning='False' the idea would be:
> D-Bus scan request or at start-up of system
> Launch an active scan
> 3 seconds -> Launch passive scan
> Stop. No more scanning: 3 seconds would be enough for ConnMan to
> reconnect to a known service in case there is one of them in range.
> New D-Bus scan request from user
> Launch an active scan
> 3 seconds -> Launch passive scan
> Stop.
I implemented a patch following above description. In addition, the
patch takes into account that ConnMan will only ask for an active scan
if user has connected to at least one WiFi network in the past and the
WiFi chipset support active scanning. If those conditions are not
true, ConnMan will always ask for a passive scan. Given that:
Current behaviour with BackgroundScanning='True' is:
D-Bus scan request or at start-up of system
Launch a passive scan
3 seconds -> Launch passive scan
..
300 seconds -> Launch passive scan
... Continue each 300 seconds unless you perform a new D-Bus scan call
which will cause the procedure to restart from 3 seconds.
With BackgroundScanning='False' the idea is:
D-Bus scan request or at start-up of system
Launch one passive scan
Stop.
New D-Bus scan request from user
Launch one passive scan
Stop.
I tested the four cases and ConnMan is doing what I described above. I
will keep the patch applied in my systems and I would like to ask
Daniel to do it too in order to check everything continue working as
it should :)
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 34c16df..206c05e 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -64,7 +64,8 @@
#define FAVORITE_MAXIMUM_RETRIES 2
#define BGSCAN_DEFAULT "simple:30:-45:300"
-#define AUTOSCAN_DEFAULT "exponential:3:300"
+#define AUTOSCAN_EXPONENTIAL "exponential:3:300"
+#define AUTOSCAN_SINGLE "single:3"
#define P2P_FIND_TIMEOUT 30
#define P2P_CONNECTION_TIMEOUT 100
@@ -83,6 +84,12 @@ enum wifi_ap_capability{
WIFI_AP_NOT_SUPPORTED = 2,
};
+enum wifi_scanning_type {
+ WIFI_SCANNING_UNKNOWN = 0,
+ WIFI_SCANNING_PASSIVE = 1,
+ WIFI_SCANNING_ACTIVE = 2,
+};
+
struct hidden_params {
char ssid[32];
unsigned int ssid_len;
@@ -143,7 +150,7 @@ struct wifi_data {
* autoscan "emulation".
*/
struct autoscan_params *autoscan;
-
+ enum wifi_scanning_type scanning_type;
GSupplicantScanParams *scan_params;
unsigned int p2p_find_timeout;
unsigned int p2p_connection_timeout;
@@ -831,13 +838,13 @@ static void reset_autoscan(struct connman_device *device)
autoscan = wifi->autoscan;
- if (autoscan->timeout == 0 && autoscan->interval == 0)
+ autoscan->interval = 0;
+
+ if (autoscan->timeout == 0)
return;
g_source_remove(autoscan->timeout);
-
autoscan->timeout = 0;
- autoscan->interval = 0;
connman_device_unref(device);
}
@@ -1363,6 +1370,21 @@ static gboolean autoscan_timeout(gpointer data)
throw_wifi_scan(wifi->device, scan_callback_hidden);
+ /*
+ * In case BackgroundScanning is disabled, interval will reach the
+ * limit exactly after the very first passive scanning. It allows
+ * to ensure at most one passive scan is performed in such cases.
+ */
+ if (!connman_setting_get_bool("BackgroundScanning") &&
+ interval == autoscan->limit) {
+ g_source_remove(autoscan->timeout);
+ autoscan->timeout = 0;
+
+ connman_device_unref(device);
+
+ return FALSE;
+ }
+
set_interval:
DBG("interval %d", interval);
@@ -1409,19 +1431,25 @@ static struct autoscan_params
*parse_autoscan_params(const char *params)
int limit;
int base;
- DBG("Emulating autoscan");
+ DBG("");
list_params = g_strsplit(params, ":", 0);
if (list_params == 0)
return NULL;
- if (g_strv_length(list_params) < 3) {
+ if (!g_strcmp0(list_params[0], "exponential") &&
+ g_strv_length(list_params) == 3) {
+ base = atoi(list_params[1]);
+ limit = atoi(list_params[2]);
+ } else if (!g_strcmp0(list_params[0], "single") &&
+ g_strv_length(list_params) == 2)
+ base = limit = atoi(list_params[1]);
+ else {
g_strfreev(list_params);
return NULL;
}
- base = atoi(list_params[1]);
- limit = atoi(list_params[2]);
+ DBG("Setup %s autoscanning", list_params[0]);
g_strfreev(list_params);
@@ -1440,10 +1468,37 @@ static struct autoscan_params
*parse_autoscan_params(const char *params)
static void setup_autoscan(struct wifi_data *wifi)
{
- if (!wifi->autoscan)
- wifi->autoscan = parse_autoscan_params(AUTOSCAN_DEFAULT);
+ /*
+ * If BackgroundScanning is enabled, setup exponential
+ * autoscanning if it has not been previously done.
+ */
+ if (connman_setting_get_bool("BackgroundScanning")) {
+ wifi->autoscan = parse_autoscan_params(AUTOSCAN_EXPONENTIAL);
+ return;
+ }
- start_autoscan(wifi->device);
+ /*
+ * On the contrary, if BackgroundScanning is disabled, update autoscan
+ * parameters based on the type of scanning that is being performed.
+ */
+ if (wifi->autoscan) {
+ g_free(wifi->autoscan);
+ wifi->autoscan = NULL;
+ }
+
+ switch (wifi->scanning_type) {
+ case WIFI_SCANNING_PASSIVE:
+ /* Do not setup autoscan. */
+ break;
+ case WIFI_SCANNING_ACTIVE:
+ /* Setup one single passive scan after active. */
+ wifi->autoscan = parse_autoscan_params(AUTOSCAN_SINGLE);
+ break;
+ case WIFI_SCANNING_UNKNOWN:
+ /* Setup autoscan in this case but we should never fall here. */
+ wifi->autoscan = parse_autoscan_params(AUTOSCAN_SINGLE);
+ break;
+ }
}
static void finalize_interface_creation(struct wifi_data *wifi)
@@ -1457,13 +1512,13 @@ static void finalize_interface_creation(struct
wifi_data *wifi)
connman_device_set_powered(wifi->device, true);
- if (!connman_setting_get_bool("BackgroundScanning"))
- return;
-
if (wifi->p2p_device)
return;
- setup_autoscan(wifi);
+ if (!wifi->autoscan)
+ setup_autoscan(wifi);
+
+ start_autoscan(wifi->device);
}
static void interface_create_callback(int result,
@@ -1691,10 +1746,29 @@ static int get_latest_connections(int max_ssids,
return num_ssids;
}
+static void wifi_update_scanner_type(struct wifi_data *wifi,
+ enum wifi_scanning_type new_type)
+{
+ DBG("");
+
+ if (!wifi || wifi->scanning_type == new_type)
+ return;
+
+ wifi->scanning_type = new_type;
+
+ setup_autoscan(wifi);
+}
+
static int wifi_scan_simple(struct connman_device *device)
{
+ struct wifi_data *wifi = connman_device_get_data(device);
+
reset_autoscan(device);
+ /* Distinguish between devices performing passive and active scanning */
+ if (wifi)
+ wifi_update_scanner_type(wifi, WIFI_SCANNING_PASSIVE);
+
return throw_wifi_scan(device, scan_callback_hidden);
}
@@ -1887,6 +1961,9 @@ static int wifi_scan(enum connman_service_type type,
}
}
+ /* Distinguish between devices performing passive and active scanning */
+ wifi_update_scanner_type(wifi, WIFI_SCANNING_ACTIVE);
+
connman_device_ref(device);
reset_autoscan(device);
Thanks,
Jose Blanquicet
------------------------------
Message: 4
Date: Sun, 1 Oct 2017 20:15:21 +0200
From: Jose Blanquicet <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: Harish Jenny K N <[email protected]>, [email protected]
Subject: Re: [PATCH v2] gsupplicant: Updates to handling of security
change
Message-ID:
<cafc8ij+hnncrfrrrxdrpng-wm-igto1hze242srwdgz-zgn...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Hi Daniel,
On Sun, Oct 1, 2017 at 3:48 PM, Daniel Wagner <[email protected]> wrote:
> Harish Jenny K N <[email protected]> writes:
>
>> Changing of security type of one BSS is resulting in removing of
>> GSupplicantNetwork and other BSSs objects in the group.
>> The corresponding entries for other BSS in the same group are not
>> removed in other hash tables like bss_mapping and interface->bss_mapping.
>> This commit only deletes the network when there is a single entry in
>> network->bss_table during change in security for one bss.
>> This commit also avoids accessing any deleted bss object when reply
>> is got from async dbus call, by deleting any pending dbus calls
>> associated with the bss object.
>
> Jose, are you happy with this version? I'll apply it when I get your
> ACK.
Yes, it is fine, you can go ahead.
Best regards,
Jose Blanquicet
------------------------------
Message: 5
Date: Sun, 1 Oct 2017 20:29:07 +0200
From: Jose Blanquicet <[email protected]>
To: Ismo Puustinen <[email protected]>
Cc: [email protected], Daniel Wagner <[email protected]>
Subject: Re: [PATCH 1/2] resolver: add support for systemd-resolved.
Message-ID:
<cafc8ijjczebupubaj5hqmysa5inwrrsgmx8xel_+hgwj2mw...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Hi Ismo,
On Sun, Oct 1, 2017 at 3:25 PM, Daniel Wagner <[email protected]> wrote:
> Hi Ismo,
>
> Ismo Puustinen <[email protected]> writes:
>
>> If systemd-resolved is running, use it for DNS address resolution. In
>> this case just deliver the DNS addresses to systemd-resolved and do not
>> run our own DNS proxy. The systemd-resolved support is selected to
>> be used if command line option "-s" is passed to connmand. This
>> implicitly disables connmand's DNS proxy.
>
> I tried this patch with 'connmand -s -n -d' and systemd-reoslvd is
> running in the background. DNS resolve didn't work anymore. Do I need to
> change anything on my setup to get this working?
>
> Anyway a few nitpicks below.
>
>> ---
>> src/connman.h | 2 +-
>> src/main.c | 5 +-
>> src/resolver.c | 234
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 229 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/connman.h b/src/connman.h
>> index 21b70802..1f7afdbc 100644
>> --- a/src/connman.h
>> +++ b/src/connman.h
>> @@ -249,7 +249,7 @@ char **__connman_inet_get_pnp_nameservers(const char
>> *pnp_file);
>>
>> #include <connman/resolver.h>
>>
>> -int __connman_resolver_init(gboolean dnsproxy);
>> +int __connman_resolver_init(gboolean dnsproxy, gboolean resolved);
>> void __connman_resolver_cleanup(void);
>> void __connman_resolver_append_fallback_nameservers(void);
>> int __connman_resolvfile_append(int index, const char *domain, const char
>> *server);
>> diff --git a/src/main.c b/src/main.c
>> index b78a0461..2cb5c56e 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -511,6 +511,7 @@ static gchar *option_noplugin = NULL;
>> static gchar *option_wifi = NULL;
>> static gboolean option_detach = TRUE;
>> static gboolean option_dnsproxy = TRUE;
>> +static gboolean option_resolved = FALSE;
>> static gboolean option_backtrace = TRUE;
>> static gboolean option_version = FALSE;
>>
>> @@ -572,6 +573,8 @@ static GOptionEntry options[] = {
>> { "nodnsproxy", 'r', G_OPTION_FLAG_REVERSE,
>> G_OPTION_ARG_NONE, &option_dnsproxy,
>> "Don't enable DNS Proxy" },
>> + { "resolvedproxy", 's', 0, G_OPTION_ARG_NONE, &option_resolved,
>> + "Use systemd-resolved for DNS management" },
>> { "nobacktrace", 0, G_OPTION_FLAG_REVERSE,
>> G_OPTION_ARG_NONE, &option_backtrace,
>> "Don't print out backtrace information" },
>> @@ -762,7 +765,7 @@ int main(int argc, char *argv[])
>>
>> __connman_plugin_init(option_plugin, option_noplugin);
>>
>> - __connman_resolver_init(option_dnsproxy);
>> + __connman_resolver_init(option_dnsproxy, option_resolved);
>> __connman_rtnl_start();
>> __connman_dhcp_init();
>> __connman_dhcpv6_init();
>> diff --git a/src/resolver.c b/src/resolver.c
>> index 75ea5ba6..62abb4a2 100644
>> --- a/src/resolver.c
>> +++ b/src/resolver.c
>> @@ -32,11 +32,15 @@
>> #include <sys/stat.h>
>> #include <resolv.h>
>> #include <netdb.h>
>> +#include <gdbus.h>
>>
>> +#include <connman/dbus.h>
>> #include "connman.h"
>>
>> #define RESOLV_CONF_STATEDIR STATEDIR"/resolv.conf"
>> #define RESOLV_CONF_ETC "/etc/resolv.conf"
>> +#define SYSTEMD_RESOLVED_SERVICE "org.freedesktop.resolve1"
>> +#define SYSTEMD_RESOLVED_PATH "/org/freedesktop/resolve1"
>>
>> #define RESOLVER_FLAG_PUBLIC (1 << 0)
>>
>> @@ -58,6 +62,9 @@ struct entry_data {
>>
>> static GSList *entry_list = NULL;
>> static bool dnsproxy_enabled = false;
>> +static DBusConnection *connection = NULL;
>> +static GDBusClient *client = NULL;
>> +static GDBusProxy *resolved_proxy = NULL;
>>
>> struct resolvfile_entry {
>> int index;
>> @@ -67,6 +74,154 @@ struct resolvfile_entry {
>>
>> static GList *resolvfile_list = NULL;
>>
>> +static void revertlink_append(DBusMessageIter *iter, void *user_data)
>> +{
>> + int *index = user_data;
>> +
>> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT32, index);
>> +}
>> +
>> +static int resolved_remove_all(int index)
>> +{
>> + gboolean result;
>> +
>> + result = g_dbus_proxy_method_call(resolved_proxy, "RevertLink",
>> + revertlink_append, NULL, &index, NULL);
>> +
>> + if (result == FALSE)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>
> We use the gbooleans 'natively', that is 'if (!val)' instead
> of 'if (val == FALSE)'
>
> Also you don't need to introduce here a local variable. Just write
>
> static int resolved_remove_all(int index)
> {
> if (!g_dbus_proxy_method_call(resolved_proxy, "RevertLink",
> revertlink_append, NULL, &index, NULL))
> return -EINVAL;
>
> return 0;
> }
>
>> +static void setlinkdns_append(DBusMessageIter *iter, void *user_data)
>> +{
>> + struct resolvfile_entry *entry = NULL;
>> + int result, *index = user_data;
>> + unsigned i;
>> + int type;
>> + char ipv4_bytes[4];
>> + char ipv6_bytes[16];
>> + GList *list;
>> +
>> + DBusMessageIter address_array, struct_array, byte_array;
>
> This shold also be part of the declartion block. No new line.
>
>> +
>> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT32, index);
>> +
>> + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "(iay)",
>> + &address_array);
>> +
>> + for (list = g_list_first(resolvfile_list); list; list =
>> g_list_next(list)) {
>
> No need for g_list_first(). The rest of this file is also using it
> without it.
>
>> + entry = list->data;
>> +
>> + DBG("setlinkdns_append: index: %d, server: %s, domain: %s",
>> + entry->index, entry->server ? entry->server :
>> "NULL",
>> + entry->domain ? entry->domain : "NULL");
>
> No need to add the setlinksdns_append, DBG() adds the function name on
> its own:
>
> connmand[23146]: src/resolver.c:setlinkdns_append() setlinkdns_append: index:
> 2, server: 192.168.154.1, domain: NULL
> connmand[23146]: src/resolver.c:setlinkdns_append() setlinkdns_append: index:
> 2, server: NULL, domain: localdomain
> connmand[23146]: src/resolver.c:setlinkdns_append() setlinkdns_append: index:
> 17, server: 192.168.154.1, domain: NULL
> connmand[23146]: src/resolver.c:setlinkdns_append() setlinkdns_append: index:
> 17, server: NULL, domain: localdomain
>
>
>> +
>> + if (entry->index != *index || entry->server == NULL)
>> + continue;
>> +
>> + dbus_message_iter_open_container(&address_array,
>> DBUS_TYPE_STRUCT, NULL,
>> + &struct_array);
>> +
>> + type = connman_inet_check_ipaddress(entry->server);
>> +
>> + if (type == AF_INET) {
>> + result = inet_pton(type, entry->server, ipv4_bytes);
>> + if (!result) {
>> + DBG("Failed to parse IPv4 address: %s",
>> entry->server);
>> + return;
>> + }
>> + dbus_message_iter_append_basic(&struct_array,
>> DBUS_TYPE_INT32,
>> + &type);
>> + dbus_message_iter_open_container(&struct_array,
>> DBUS_TYPE_ARRAY,
>> + "y", &byte_array);
>> + for (i = 0; i < sizeof(ipv4_bytes); i++) {
>> + dbus_message_iter_append_basic(&byte_array,
>> DBUS_TYPE_BYTE,
>> + &(ipv4_bytes[i]));
>> + }
>> + dbus_message_iter_close_container(&struct_array,
>> &byte_array);
>> + }
>> + else {
>
> indention should be
>
> } else {
>
> but I rather have an explicit test to 'if (type == AF_INET6)'
>
> I suggest to move the bodies of the if condition into a function. The
> lines are too long and the function is a bit too long.
>
>
>> + result = inet_pton(type, entry->server, ipv6_bytes);
>> + if (!result) {
>> + DBG("Failed to parse IPv4 address: %s",
>> entry->server);
>> + return;
>> + }
>> + dbus_message_iter_append_basic(&struct_array,
>> DBUS_TYPE_INT32,
>> + &type);
>> + dbus_message_iter_open_container(&struct_array,
>> DBUS_TYPE_ARRAY,
>> + "y", &byte_array);
>> + for (i = 0; i < sizeof(ipv6_bytes); i++) {
>> + dbus_message_iter_append_basic(&byte_array,
>> DBUS_TYPE_BYTE,
>> + &(ipv6_bytes[i]));
>> + }
>> + dbus_message_iter_close_container(&struct_array,
>> &byte_array);
>> + }
>> + dbus_message_iter_close_container(&address_array,
>> &struct_array);
>> + }
>> +
>> + dbus_message_iter_close_container(iter, &address_array);
>> +}
>
> You should also add some new lines.
>
>> +
>> +static void setlinkdomains_append(DBusMessageIter *iter, void *user_data)
>> +{
>> + struct resolvfile_entry *entry;
>> + int *index = user_data;
>> + DBusMessageIter domain_array, struct_array;
>> + gboolean only_routing = FALSE;
>> + GList *list;
>> +
>> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT32, index);
>> +
>> + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "(sb)",
>> + &domain_array);
>> +
>> + for (list = g_list_first(resolvfile_list); list; list =
>> g_list_next(list)) {
>> + entry = list->data;
>> +
>> + DBG("setlinkdomains_append: index: %d, server: %s, domain: %s",
>> + entry->index, entry->server ? entry->server :
>> "NULL",
>> + entry->domain ? entry->domain : "NULL");
>> +
>
> No function name needed as prefix in the DBG().
>
>> + if (entry->index != *index || entry->domain == NULL)
>> + continue;
>> +
>> + dbus_message_iter_open_container(&domain_array,
>> DBUS_TYPE_STRUCT, NULL,
>> + &struct_array);
>> +
>> + dbus_message_iter_append_basic(&struct_array, DBUS_TYPE_STRING,
>> + &entry->domain);
>> + dbus_message_iter_append_basic(&struct_array,
>> DBUS_TYPE_BOOLEAN,
>> + &only_routing);
>> +
>> + dbus_message_iter_close_container(&domain_array,
>> &struct_array);
>> + }
>> + dbus_message_iter_close_container(iter, &domain_array);
>> +}
>> +
>> +static int resolved_export(int index)
>> +{
>> + gboolean result;
>> +
>> + if (!resolved_proxy)
>> + return -ENOENT;
>> +
>> + /* No async error processing -- just fire and forget */
>> +
>> + result = g_dbus_proxy_method_call(resolved_proxy, "SetLinkDNS",
>> + setlinkdns_append, NULL, &index, NULL);
>> + if (result == FALSE)
>> + return -EINVAL;
>
> if (!g_dbus_proxy_method_call(...))
> return -EINVAL;
>
>
>> +
>> + result = g_dbus_proxy_method_call(resolved_proxy, "SetLinkDomains",
>> + setlinkdomains_append, NULL, &index, NULL);
>> + if (result == FALSE)
>> + return -EINVAL;
>
> same here
>
>> +
>> + return 0;
>> +}
>> +
>> static void resolvfile_remove_entries(GList *entries)
>> {
>> GList *list;
>> @@ -188,11 +343,14 @@ int __connman_resolvfile_append(int index, const char
>> *domain,
>>
>> resolvfile_list = g_list_append(resolvfile_list, entry);
>>
>> + if (resolved_proxy)
>> + return resolved_export(index);
>> +
>> return resolvfile_export();
>> }
>>
>> -int __connman_resolvfile_remove(int index, const char *domain,
>> - const char *server)
>> +static int __connman_resolvfile_remove_entry(int index, const char *domain,
>> + const char *server,
>> gboolean remove_from_resolved)
>
> Use bool here instead of gboolean. We only use gboolean where we have to
> because of glib. Internally we just use C99 bools.
>
> And the line is too long.
>
>> {
>> GList *list, *matches = NULL;
>>
>> @@ -215,9 +373,22 @@ int __connman_resolvfile_remove(int index, const char
>> *domain,
>>
>> resolvfile_remove_entries(matches);
>>
>> + if (resolved_proxy) {
>> + if (remove_from_resolved)
>> + return resolved_export(index);
>> + else
>> + return 0; /* whole link is reset later */
>
> The 'else' is useless.
>
> if (remove_from_resolved)
> return resolved_export(index);
>
> return 0; /* whole link is reset later */
>
>
>> + }
>> +
>> return resolvfile_export();
>> }
>>
>> +int __connman_resolvfile_remove(int index, const char *domain,
>> + const char *server)
>> +{
>> + return __connman_resolvfile_remove_entry(index, domain, server, TRUE);
>> +}
>> +
>> void __connman_resolver_append_fallback_nameservers(void)
>> {
>> GSList *list;
>> @@ -269,7 +440,7 @@ static void remove_fallback_nameservers(void)
>> }
>> }
>>
>> -static void remove_entries(GSList *entries)
>> +static void remove_entries(GSList *entries, int index)
>> {
>> GSList *list;
>>
>> @@ -282,8 +453,8 @@ static void remove_entries(GSList *entries)
>> __connman_dnsproxy_remove(entry->index, entry->domain,
>> entry->server);
>> } else {
>> - __connman_resolvfile_remove(entry->index,
>> entry->domain,
>> - entry->server);
>> + __connman_resolvfile_remove_entry(entry->index,
>> entry->domain,
>> + entry->server, index <
>> 0 ? TRUE : FALSE);
>
> You can just write '..., index < 0)' that make the line short enough to
> fit below the 80 chars limit.
>
>> }
>>
>> if (entry->timeout)
>> @@ -295,6 +466,9 @@ static void remove_entries(GSList *entries)
>>
>> g_slist_free(entries);
>>
>> + if (resolved_proxy && index >= 0)
>> + resolved_remove_all(index);
>> +
>> __connman_resolver_append_fallback_nameservers();
>> }
>>
>> @@ -316,7 +490,7 @@ static gboolean resolver_expire_cb(gpointer user_data)
>> entry->server, true);
>> }
>>
>> - remove_entries(list);
>> + remove_entries(list, -1);
>>
>> return FALSE;
>> }
>> @@ -538,7 +712,7 @@ int connman_resolver_remove(int index, const char
>> *domain, const char *server)
>> if (!matches)
>> return -ENOENT;
>>
>> - remove_entries(matches);
>> + remove_entries(matches, -1);
>>
>> return 0;
>> }
>> @@ -570,7 +744,10 @@ int connman_resolver_remove_all(int index)
>> if (!matches)
>> return -ENOENT;
>>
>> - remove_entries(matches);
>> + if (resolved_proxy)
>> + return resolved_remove_all(index);
>> +
>> + remove_entries(matches, index);
>>
>> return 0;
>> }
>> @@ -652,12 +829,34 @@ static void free_resolvfile(gpointer data)
>> g_free(entry);
>> }
>>
>> -int __connman_resolver_init(gboolean dnsproxy)
>> +int __connman_resolver_init(gboolean dnsproxy, gboolean resolved)
>> {
>> int i;
>> char **ns;
>>
>> - DBG("dnsproxy %d", dnsproxy);
>> + DBG("dnsproxy %d, resolved %d", dnsproxy, resolved);
>> +
>> + if (resolved) {
>> + connection = connman_dbus_get_connection();
>> +
>> + if (connection) {
>> + client = g_dbus_client_new(connection,
>> + SYSTEMD_RESOLVED_SERVICE,
>> SYSTEMD_RESOLVED_PATH);
>> + if (!client)
>> + DBG("Failed to initialize proxy for
>> systemd-resolved");
>
> Is this a proper error? In this case use connman_warn() or
> connman_error().
>
>> + else {
>> + resolved_proxy = g_dbus_proxy_new(client,
>> SYSTEMD_RESOLVED_PATH,
>> +
>> "org.freedesktop.resolve1.Manager");
>> +
>> + if (resolved_proxy) {
>> + /* setup succeeded, disable dnsproxy */
>> + dnsproxy = FALSE;
>> + }
>> + else
>
> } else {
>
>> + DBG("Failed to initialize proxy for
>> systemd-resolved");
>
> Same here.
>
> And the lines are a bit long.
I just want to suggest you to run linux's checkpatch on the patches.
It will point out most of coding style problems Daniel remarked.
Best regards,
Jose Blanquicet
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 24, Issue 2
**************************************
