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. [PATCH][v2] tethering: Add verification to bridge creation
and configuration (Jose Blanquicet)
2. [PATCH 1/3] tethering: Propagate error value when enabling
tethering (Jose Blanquicet)
3. [PATCH 2/3] technology: Handle errors when enabling tethering
(Jose Blanquicet)
4. [PATCH 3/3] wifi: Abort tethering enabling if bridge
creation/configuration fails (Jose Blanquicet)
5. [PATCH] stats: Fix bad file descriptor initialisation
(Lukasz Nowak)
----------------------------------------------------------------------
Message: 1
Date: Mon, 24 Oct 2016 14:21:06 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected]
Subject: [PATCH][v2] tethering: Add verification to bridge creation
and configuration
Message-ID: <[email protected]>
Version 2:
- Return 0 (No error) in case __sync_fetch_and_add(&tethering_enabled, 1) != 0.
- Split up PATCH v1 into multiple patches as requested.
As I told before, use this verification in others tethering technologies should
be evaluated and implemented in case it is advantageous. I will let this in the
hands of someone with more expertise/knowledge on those other technologies. I
only focused on WiFi. Should I add also the TODO comments into the other plugins
as I had done for v1?
Jose Blanquicet (3):
tethering: Propagate error value when enabling tethering
technology: Handle errors when enabling tethering
wifi: Abort tethering enabling if bridge creation/configuration fails
include/technology.h | 2 +-
plugins/wifi.c | 23 ++++++++++++++++++-----
src/connman.h | 2 +-
src/technology.c | 19 ++++++++++++-------
src/tethering.c | 16 +++++++++-------
5 files changed, 41 insertions(+), 21 deletions(-)
--
1.9.1
------------------------------
Message: 2
Date: Mon, 24 Oct 2016 14:21:07 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected]
Subject: [PATCH 1/3] tethering: Propagate error value when enabling
tethering
Message-ID: <[email protected]>
Add return value to __connman_tethering_set_enabled() in order to propagate
errors and allow connman_technology_tethering_notify() to not notify tethering
as enabled when actually it does not because bridge creation fails.
---
src/connman.h | 2 +-
src/tethering.c | 16 +++++++++-------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index 401e3d7..ce3ef8d 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -620,7 +620,7 @@ int __connman_tethering_init(void);
void __connman_tethering_cleanup(void);
const char *__connman_tethering_get_bridge(void);
-void __connman_tethering_set_enabled(void);
+int __connman_tethering_set_enabled(void);
void __connman_tethering_set_disabled(void);
int __connman_private_network_request(DBusMessage *msg, const char *owner);
diff --git a/src/tethering.c b/src/tethering.c
index 3153349..c929ba7 100644
--- a/src/tethering.c
+++ b/src/tethering.c
@@ -181,7 +181,7 @@ static void tethering_restart(struct connman_ippool *pool,
void *user_data)
__connman_tethering_set_enabled();
}
-void __connman_tethering_set_enabled(void)
+int __connman_tethering_set_enabled(void)
{
int index;
int err;
@@ -197,12 +197,12 @@ void __connman_tethering_set_enabled(void)
DBG("enabled %d", tethering_enabled + 1);
if (__sync_fetch_and_add(&tethering_enabled, 1) != 0)
- return;
+ return 0;
err = __connman_bridge_create(BRIDGE_NAME);
if (err < 0) {
__sync_fetch_and_sub(&tethering_enabled, 1);
- return;
+ return -EOPNOTSUPP;
}
index = connman_inet_ifindex(BRIDGE_NAME);
@@ -212,7 +212,7 @@ void __connman_tethering_set_enabled(void)
connman_error("Fail to create IP pool");
__connman_bridge_remove(BRIDGE_NAME);
__sync_fetch_and_sub(&tethering_enabled, 1);
- return;
+ return -EADDRNOTAVAIL;
}
gateway = __connman_ippool_get_gateway(dhcp_ippool);
@@ -228,7 +228,7 @@ void __connman_tethering_set_enabled(void)
__connman_ippool_unref(dhcp_ippool);
__connman_bridge_remove(BRIDGE_NAME);
__sync_fetch_and_sub(&tethering_enabled, 1);
- return;
+ return -EADDRNOTAVAIL;
}
ns = connman_setting_get_string_list("FallbackNameservers");
@@ -264,7 +264,7 @@ void __connman_tethering_set_enabled(void)
__connman_ippool_unref(dhcp_ippool);
__connman_bridge_remove(BRIDGE_NAME);
__sync_fetch_and_sub(&tethering_enabled, 1);
- return;
+ return -EOPNOTSUPP;
}
prefixlen = connman_ipaddress_calc_netmask_len(subnet_mask);
@@ -276,7 +276,7 @@ void __connman_tethering_set_enabled(void)
__connman_ippool_unref(dhcp_ippool);
__connman_bridge_remove(BRIDGE_NAME);
__sync_fetch_and_sub(&tethering_enabled, 1);
- return;
+ return -EOPNOTSUPP;
}
err = __connman_ipv6pd_setup(BRIDGE_NAME);
@@ -285,6 +285,8 @@ void __connman_tethering_set_enabled(void)
strerror(-err));
DBG("tethering started");
+
+ return 0;
}
void __connman_tethering_set_disabled(void)
--
1.9.1
------------------------------
Message: 3
Date: Mon, 24 Oct 2016 14:21:08 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected]
Subject: [PATCH 2/3] technology: Handle errors when enabling tethering
Message-ID: <[email protected]>
Use __connman_tethering_set_enabled()'s return value to verify if there was
an error while creating/configuring the bridge in order to not wrongly notify
that tethering was enabled and allow plugins to abort tethering enabling in
case of something was wrong.
---
include/technology.h | 2 +-
src/technology.c | 19 ++++++++++++-------
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/technology.h b/include/technology.h
index d7fcdde..97db660 100644
--- a/include/technology.h
+++ b/include/technology.h
@@ -36,7 +36,7 @@ extern "C" {
struct connman_technology;
-void connman_technology_tethering_notify(struct connman_technology *technology,
+int connman_technology_tethering_notify(struct connman_technology *technology,
bool enabled);
int connman_technology_set_regdom(const char *alpha2);
void connman_technology_regdom_notify(struct connman_technology *technology,
diff --git a/src/technology.c b/src/technology.c
index 660af52..6f91af7 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -211,22 +211,27 @@ static void tethering_changed(struct connman_technology
*technology)
technology_save(technology);
}
-void connman_technology_tethering_notify(struct connman_technology *technology,
+int connman_technology_tethering_notify(struct connman_technology *technology,
bool enabled)
{
+ int err;
+
DBG("technology %p enabled %u", technology, enabled);
if (technology->tethering == enabled)
- return;
+ return -EALREADY;
- technology->tethering = enabled;
+ if (enabled) {
+ err = __connman_tethering_set_enabled();
+ if (err < 0)
+ return err;
+ } else
+ __connman_tethering_set_disabled();
+ technology->tethering = enabled;
tethering_changed(technology);
- if (enabled)
- __connman_tethering_set_enabled();
- else
- __connman_tethering_set_disabled();
+ return 0;
}
static int set_tethering(struct connman_technology *technology,
--
1.9.1
------------------------------
Message: 4
Date: Mon, 24 Oct 2016 14:21:09 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected]
Subject: [PATCH 3/3] wifi: Abort tethering enabling if bridge
creation/configuration fails
Message-ID: <[email protected]>
In case there is an error while setting up the bridge for WiFi tethering, the
selected interface for tethering will remain removed and could not be use
anymore. This patch aims to avoid such a situation by using
connman_technology_tethering_notify()'s return value to abort tethering
enabling thus do not unnecessary remove the interface in case of something was
wrong with bridge.
---
plugins/wifi.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 8bde9c0..1b25739 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -3138,13 +3138,14 @@ static void sta_remove_callback(int result,
DBG("ifname %s result %d ", info->ifname, result);
if ((result < 0) || (info->wifi->ap_supported != WIFI_AP_SUPPORTED)) {
- info->wifi->tethering = true;
+ info->wifi->tethering = false;
+ connman_technology_tethering_notify(info->technology, false);
g_free(info->ifname);
g_free(info->ssid);
g_free(info);
- if (info->wifi->ap_supported == WIFI_AP_SUPPORTED){
+ if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
g_free(info->wifi->tethering_param->ssid);
g_free(info->wifi->tethering_param);
info->wifi->tethering_param = NULL;
@@ -3154,8 +3155,6 @@ static void sta_remove_callback(int result,
info->wifi->interface = NULL;
- connman_technology_tethering_notify(info->technology, true);
-
g_supplicant_interface_create(info->ifname, driver, info->wifi->bridge,
ap_create_callback,
info);
@@ -3171,7 +3170,7 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
struct wifi_tethering_info *info;
const char *ifname;
unsigned int mode;
- int err;
+ int err, bridge_result = 0;
for (list = iface_list; list; list = list->next) {
wifi = list->data;
@@ -3230,6 +3229,11 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
info->wifi->tethering = true;
info->wifi->ap_supported = WIFI_AP_SUPPORTED;
+ bridge_result =
+ connman_technology_tethering_notify(technology, true);
+ if (bridge_result < 0)
+ goto failed;
+
err = g_supplicant_interface_remove(interface,
sta_remove_callback,
info);
@@ -3244,6 +3248,15 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
g_free(info);
g_free(wifi->tethering_param);
wifi->tethering_param = NULL;
+
+ if (bridge_result < 0) {
+ /*
+ * If it was an error while setting up the bridge then
+ * do not try again on another interface, bridge set-up
+ * does not depend on it.
+ */
+ break;
+ }
}
return -EOPNOTSUPP;
--
1.9.1
------------------------------
Message: 5
Date: Mon, 24 Oct 2016 16:39:40 +0100
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH] stats: Fix bad file descriptor initialisation
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
From: Lukasz Nowak <[email protected]>
Stats file code initialises its file descriptor field to 0.
But 0 is a valid fd value. -1 should be used instead.
This causes problems when an error happens before a stats file is open
(e.g. mkdir fails). The clean-up procedure, stats_free() calls close(fd).
When fd is 0, this first closes stdin, and then any files/sockets which
received fd=0, re-used by the OS.
Fixed several instances of bad file descriptor field handling,
in case of errors.
Signed-off-by: Lukasz Nowak <[email protected]>
Reviewed-by: Andr? Draszik <[email protected]>
---
src/stats.c | 45 +++++++++++++++++++++++++++++++++++----------
src/util.c | 4 ++--
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/src/stats.c b/src/stats.c
index 26343b1..d1ec60f 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -222,12 +222,15 @@ static void stats_free(gpointer user_data)
if (!file)
return;
- msync(file->addr, file->len, MS_SYNC);
-
- munmap(file->addr, file->len);
+ if ((file->addr) && (file->len > 0))
+ {
+ msync(file->addr, file->len, MS_SYNC);
+ munmap(file->addr, file->len);
+ }
file->addr = NULL;
- TFR(close(file->fd));
+ if (file->fd >= 0)
+ TFR(close(file->fd));
file->fd = -1;
if (file->history_name) {
@@ -378,6 +381,7 @@ static int stats_file_setup(struct stats_file *file)
strerror(errno), file->name);
TFR(close(file->fd));
+ file->fd = -1;
g_free(file->name);
file->name = NULL;
@@ -393,6 +397,7 @@ static int stats_file_setup(struct stats_file *file)
err = stats_file_remap(file, size);
if (err < 0) {
TFR(close(file->fd));
+ file->fd = -1;
g_free(file->name);
file->name = NULL;
@@ -601,8 +606,11 @@ static int summarize(struct stats_file *data_file,
static void stats_file_unmap(struct stats_file *file)
{
- msync(file->addr, file->len, MS_SYNC);
- munmap(file->addr, file->len);
+ if ((file->addr) && (file->len > 0))
+ {
+ msync(file->addr, file->len, MS_SYNC);
+ munmap(file->addr, file->len);
+ }
file->addr = NULL;
}
@@ -621,15 +629,19 @@ static int stats_file_close_swap(struct stats_file
*history_file,
stats_file_unmap(history_file);
stats_file_unmap(temp_file);
- TFR(close(temp_file->fd));
+ if (temp_file->fd >= 0)
+ TFR(close(temp_file->fd));
- unlink(history_file->name);
+ if (history_file->name)
+ unlink(history_file->name);
err = link(temp_file->name, history_file->name);
- unlink(temp_file->name);
+ if (temp_file->name)
+ unlink(temp_file->name);
- TFR(close(history_file->fd));
+ if (history_file->fd >= 0)
+ TFR(close(history_file->fd));
stats_file_cleanup(history_file);
stats_file_cleanup(temp_file);
@@ -649,6 +661,13 @@ static int stats_file_history_update(struct stats_file
*data_file)
bzero(history_file, sizeof(struct stats_file));
bzero(temp_file, sizeof(struct stats_file));
+ /*
+ * 0 is a valid file descriptor - fd needs to be initialized
+ * to -1 to handle errors correctly
+ */
+ history_file->fd = -1;
+ temp_file->fd = -1;
+
err = stats_open(history_file, data_file->history_name);
if (err < 0)
return err;
@@ -682,6 +701,12 @@ int __connman_stats_service_register(struct
connman_service *service)
if (!file)
return -ENOMEM;
+ /*
+ * 0 is a valid file descriptor - fd needs to be initialized
+ * to -1 to handle errors correctly
+ */
+ file->fd = -1;
+
g_hash_table_insert(stats_hash, service, file);
} else {
return -EALREADY;
diff --git a/src/util.c b/src/util.c
index da32cc5..0a4413f 100644
--- a/src/util.c
+++ b/src/util.c
@@ -58,7 +58,7 @@ int __connman_util_init(void)
{
int r = 0;
- if (f > 0)
+ if (f >= 0)
return 0;
f = open(URANDOM, O_RDONLY);
@@ -81,7 +81,7 @@ int __connman_util_init(void)
void __connman_util_cleanup(void)
{
- if (f > 0)
+ if (f >= 0)
close(f);
f = -1;
--
2.7.4
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 12, Issue 24
***************************************
